Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

localstore reserve logic #1322

Merged
merged 33 commits into from
Apr 13, 2021
Merged

localstore reserve logic #1322

merged 33 commits into from
Apr 13, 2021

Conversation

zelig
Copy link
Member

@zelig zelig commented Feb 24, 2021

the pr also fixes an index leak which happened (ModeGetRequest) when an as yet unsynced chunk was requested. It was put to GC incorrectly but moreover this entry was not removed when the chunk was synced (setSync was called)

#1346

AP https://hackmd.io/9ejVZEvSQg69zXG91YsFEQ

@zelig zelig added the builds on open pr builds on another open one label Feb 24, 2021
@zelig zelig self-assigned this Feb 24, 2021
@zelig zelig force-pushed the postage-stamp-storage branch 2 times, most recently from 1694c83 to c020c7a Compare February 24, 2021 02:37
@zelig zelig force-pushed the postage-localstore-reserve branch 4 times, most recently from 229d470 to a51dc04 Compare February 24, 2021 14:36
@zelig zelig force-pushed the postage-stamp-storage branch 2 times, most recently from f850603 to 435d47b Compare March 15, 2021 01:01
@zelig zelig force-pushed the postage-localstore-reserve branch from a51dc04 to 9b978c0 Compare March 15, 2021 21:08
@acud acud force-pushed the postage-localstore-reserve branch 2 times, most recently from db8ddaa to 8cecb25 Compare April 2, 2021 11:58
pkg/localstore/localstore.go Outdated Show resolved Hide resolved
Base automatically changed from postage-stamp-storage to storage-incentives April 2, 2021 12:11
@acud acud force-pushed the postage-localstore-reserve branch from b98122c to bfc6fcc Compare April 2, 2021 12:18
@acud acud removed the builds on open pr builds on another open one label Apr 2, 2021
pkg/localstore/postage.go Outdated Show resolved Hide resolved
pkg/node/node.go Show resolved Hide resolved
@acud acud force-pushed the postage-localstore-reserve branch 4 times, most recently from 83e4fdc to 32330ce Compare April 9, 2021 13:37
@acud acud added ready for review The PR is ready to be reviewed and removed pull-request labels Apr 9, 2021
@acud acud force-pushed the postage-localstore-reserve branch from 4506b4d to 7bc51be Compare April 10, 2021 17:21
}
item.Radius = item2.Radius

if withinRadiusFn(db, item) || forcePin {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when this is from a simple setsync call we also should not set the access timestamp to now.
only if it is putRequest.

but surely not ike this, instead:
make access update consistent irrespective of reserve/cache status. (as reserve can be evicted after a second and then will be deleted even before their earlier cached (and earlier accessed) sister chunks.

pkg/localstore/reserve.go Show resolved Hide resolved
if err != nil {
return false, err
}
err = db.pullIndex.DeleteInBatch(batch, item2)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not consistent. The pullindex must include chunk batches otherwise nothing will be in the pullindex from any bin outside the neighbourhood radius

pkg/localstore/localstore.go Outdated Show resolved Hide resolved
pkg/localstore/localstore.go Outdated Show resolved Hide resolved
pkg/localstore/mode_get.go Show resolved Hide resolved
return false, 0, err
}

item.AccessTimestamp = now()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: this is to be.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

pkg/localstore/mode_put.go Outdated Show resolved Hide resolved
pkg/localstore/mode_put.go Outdated Show resolved Hide resolved
}
if !ok {
err = db.gcIndex.PutInBatch(batch, item)
item.AccessTimestamp = now()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: here please take note of this. This should leave the access timestamp alone.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the following line also dropped

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

....don't understand.... i thought you said setSync should update access timestamp

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's please have a writeup or at least a truth table about what happens to which index under which circumstances. i find documenting this in the code not the best thing to do and it just pollutes the code with hypothetical reminders. let's start with defining, then make changes to the codebase and in the meanwhile try to keep it clean

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it is quite simple, you update accesstimestamp if and only if the chunk iis requested

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

truth table?

pkg/localstore/reserve_test.go Show resolved Hide resolved
Copy link
Member Author

@zelig zelig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@acud
Copy link
Member

acud commented Apr 13, 2021

famous last words?

@acud acud merged commit 2ee3530 into storage-incentives Apr 13, 2021
@acud acud deleted the postage-localstore-reserve branch April 13, 2021 11:30
acud added a commit that referenced this pull request Apr 13, 2021
postage: new pkg for postage stamps, uploader stamping (#890)

* postage: new pkg for postage stamps, uploader stamping

* postage: amount->value, blockNumber big.Int-> uint64, stamp only has batch ID, not Batch

* postage: fix godoc and copyright

* postage, swarm:
 - swarm.Stamp as an interface
 - add postage/testing for mock Stamps
 - fix Stamp MarshalBinary to allow nil batch id and signature
 - add StampSize const

* postage: heed review feedback

Co-authored-by: acud <[email protected]>

Storage incentives: add stamper putter to api

Postage BatchStore and BatchService (#1070)

Co-authored-by: zelig <[email protected]>

add normalisedBalance to updater interface (#1108)

make value the normalised balance (#1111)

postage: add event listener (#1099)

Wire up postage stamp syncing (#1114)

localstore, shed: persist stamps (#1116)

add --postage to beeinfra.sh setup

pullsync, pushsync: add postage stamps (#1117)

postage: add create endpoint (#1142)

retrieve erc20 address from postage contract (#1169)

postage: check balance before attempting stamp creation (#1177)

postage: fix bucket depth (#1178)

api: use hex encoding in postage api (#1179)

increase page size (#1182)

postage: handle bucket depth error in api (#1183)

localstore: attach stamp to outgoing chunk (#1192)

update postage stamp contract addresses for new token (#1208)

batchstore: reserve (#1262)

* postage/batchstore: reserve logic

Co-authored-by: acud <[email protected]>

stamp support in storage and protocols (#1321)

api: endpoints for stamp issuers (#1535)

retrieval: add stamps (#1552)

localstore reserve logic (#1322)

Co-authored-by: acud <[email protected]>
acud added a commit that referenced this pull request Apr 15, 2021
postage: new pkg for postage stamps, uploader stamping (#890)

* postage: new pkg for postage stamps, uploader stamping

* postage: amount->value, blockNumber big.Int-> uint64, stamp only has batch ID, not Batch

* postage: fix godoc and copyright

* postage, swarm:
 - swarm.Stamp as an interface
 - add postage/testing for mock Stamps
 - fix Stamp MarshalBinary to allow nil batch id and signature
 - add StampSize const

* postage: heed review feedback

Co-authored-by: acud <[email protected]>

Storage incentives: add stamper putter to api

Postage BatchStore and BatchService (#1070)

Co-authored-by: zelig <[email protected]>

add normalisedBalance to updater interface (#1108)

make value the normalised balance (#1111)

postage: add event listener (#1099)

Wire up postage stamp syncing (#1114)

localstore, shed: persist stamps (#1116)

add --postage to beeinfra.sh setup

pullsync, pushsync: add postage stamps (#1117)

postage: add create endpoint (#1142)

retrieve erc20 address from postage contract (#1169)

postage: check balance before attempting stamp creation (#1177)

postage: fix bucket depth (#1178)

api: use hex encoding in postage api (#1179)

increase page size (#1182)

postage: handle bucket depth error in api (#1183)

localstore: attach stamp to outgoing chunk (#1192)

update postage stamp contract addresses for new token (#1208)

batchstore: reserve (#1262)

* postage/batchstore: reserve logic

Co-authored-by: acud <[email protected]>

stamp support in storage and protocols (#1321)

api: endpoints for stamp issuers (#1535)

retrieval: add stamps (#1552)

localstore reserve logic (#1322)

Co-authored-by: acud <[email protected]>
acud added a commit that referenced this pull request Apr 26, 2021
postage: new pkg for postage stamps, uploader stamping (#890)

* postage: new pkg for postage stamps, uploader stamping

* postage: amount->value, blockNumber big.Int-> uint64, stamp only has batch ID, not Batch

* postage: fix godoc and copyright

* postage, swarm:
 - swarm.Stamp as an interface
 - add postage/testing for mock Stamps
 - fix Stamp MarshalBinary to allow nil batch id and signature
 - add StampSize const

* postage: heed review feedback

Co-authored-by: acud <[email protected]>

Storage incentives: add stamper putter to api

Postage BatchStore and BatchService (#1070)

Co-authored-by: zelig <[email protected]>

add normalisedBalance to updater interface (#1108)

make value the normalised balance (#1111)

postage: add event listener (#1099)

Wire up postage stamp syncing (#1114)

localstore, shed: persist stamps (#1116)

add --postage to beeinfra.sh setup

pullsync, pushsync: add postage stamps (#1117)

postage: add create endpoint (#1142)

retrieve erc20 address from postage contract (#1169)

postage: check balance before attempting stamp creation (#1177)

postage: fix bucket depth (#1178)

api: use hex encoding in postage api (#1179)

increase page size (#1182)

postage: handle bucket depth error in api (#1183)

localstore: attach stamp to outgoing chunk (#1192)

update postage stamp contract addresses for new token (#1208)

batchstore: reserve (#1262)

* postage/batchstore: reserve logic

Co-authored-by: acud <[email protected]>

stamp support in storage and protocols (#1321)

api: endpoints for stamp issuers (#1535)

retrieval: add stamps (#1552)

localstore reserve logic (#1322)

Co-authored-by: acud <[email protected]>
acud added a commit that referenced this pull request Apr 27, 2021
postage: new pkg for postage stamps, uploader stamping (#890)

* postage: new pkg for postage stamps, uploader stamping

* postage: amount->value, blockNumber big.Int-> uint64, stamp only has batch ID, not Batch

* postage: fix godoc and copyright

* postage, swarm:
 - swarm.Stamp as an interface
 - add postage/testing for mock Stamps
 - fix Stamp MarshalBinary to allow nil batch id and signature
 - add StampSize const

* postage: heed review feedback

Co-authored-by: acud <[email protected]>

Storage incentives: add stamper putter to api

Postage BatchStore and BatchService (#1070)

Co-authored-by: zelig <[email protected]>

add normalisedBalance to updater interface (#1108)

make value the normalised balance (#1111)

postage: add event listener (#1099)

Wire up postage stamp syncing (#1114)

localstore, shed: persist stamps (#1116)

add --postage to beeinfra.sh setup

pullsync, pushsync: add postage stamps (#1117)

postage: add create endpoint (#1142)

retrieve erc20 address from postage contract (#1169)

postage: check balance before attempting stamp creation (#1177)

postage: fix bucket depth (#1178)

api: use hex encoding in postage api (#1179)

increase page size (#1182)

postage: handle bucket depth error in api (#1183)

localstore: attach stamp to outgoing chunk (#1192)

update postage stamp contract addresses for new token (#1208)

batchstore: reserve (#1262)

* postage/batchstore: reserve logic

Co-authored-by: acud <[email protected]>

stamp support in storage and protocols (#1321)

api: endpoints for stamp issuers (#1535)

retrieval: add stamps (#1552)

localstore reserve logic (#1322)

Co-authored-by: acud <[email protected]>
acud added a commit that referenced this pull request Apr 27, 2021
postage: new pkg for postage stamps, uploader stamping (#890)

* postage: new pkg for postage stamps, uploader stamping

* postage: amount->value, blockNumber big.Int-> uint64, stamp only has batch ID, not Batch

* postage: fix godoc and copyright

* postage, swarm:
 - swarm.Stamp as an interface
 - add postage/testing for mock Stamps
 - fix Stamp MarshalBinary to allow nil batch id and signature
 - add StampSize const

* postage: heed review feedback

Co-authored-by: acud <[email protected]>

Storage incentives: add stamper putter to api

Postage BatchStore and BatchService (#1070)

Co-authored-by: zelig <[email protected]>

add normalisedBalance to updater interface (#1108)

make value the normalised balance (#1111)

postage: add event listener (#1099)

Wire up postage stamp syncing (#1114)

localstore, shed: persist stamps (#1116)

add --postage to beeinfra.sh setup

pullsync, pushsync: add postage stamps (#1117)

postage: add create endpoint (#1142)

retrieve erc20 address from postage contract (#1169)

postage: check balance before attempting stamp creation (#1177)

postage: fix bucket depth (#1178)

api: use hex encoding in postage api (#1179)

increase page size (#1182)

postage: handle bucket depth error in api (#1183)

localstore: attach stamp to outgoing chunk (#1192)

update postage stamp contract addresses for new token (#1208)

batchstore: reserve (#1262)

* postage/batchstore: reserve logic

Co-authored-by: acud <[email protected]>

stamp support in storage and protocols (#1321)

api: endpoints for stamp issuers (#1535)

retrieval: add stamps (#1552)

localstore reserve logic (#1322)

Co-authored-by: acud <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull-request ready for review The PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants