Add referencing for safe removal of unused Bitswap blocks #16073
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
svv232
changed the title
svv232
changed the title
|
!ci-build-me |
2 similar comments
|
!ci-build-me |
|
!ci-build-me |
|
!ci-build-me |
3 similar comments
|
!ci-build-me |
|
!ci-build-me |
|
!ci-build-me |
georgeee
reviewed
Sep 20, 2024
| @@ -14,7 +14,7 @@ libp2p_helper: ../../libp2p_ipc/libp2p_ipc.capnp.go | |||
| test: ../../libp2p_ipc/libp2p_ipc.capnp.go | |||
| cd src/libp2p_helper \ | |||
| && (ulimit -n 65536 || true) \ | |||
| && $(GO) test -short -timeout 40m | |||
| && $(GO) test -short -timeout 60m | |||
There was a problem hiding this comment.
wow, that is short 😁
is this timeout justified by having to run more tests?
could it have been accidentally leaked from development process?
There was a problem hiding this comment.
The 40 minute timeout wasn't enough for these changes with some runs. I found relatively low variance with a 60 second timeout. I wasn't sure what the problem was initially on remote until i got the test passing locally after waiting longer. We run The new downloader test as well as the util test. It seams justified to me, because the downloader was not required before.
georgeee
force-pushed
the
sai/bitswaplip2phelperdefinitions
branch
3 times, most recently
from
ff8a39d
to
63f4c3c
Compare
georgeee
force-pushed
the
sai/bitswaplibp2p
branch
from
609c254
to
b2796f7
Compare
georgeee
force-pushed
the
sai/bitswaplip2phelperdefinitions
branch
6 times, most recently
from
7a6392d
to
7df326d
Compare
Motivation: block size that we';ll be using in production is too large for effective bug searching via property tests.
Commit doesn't compile. Commit that comes next in the branch is going to fix it. Separation is made for greater clarity during review. Integrates commit o1-labs/go-bs-lmdb@1dc365f which introduces a method to atomically delete blocks after checking a predicate (`DeleteBlocksIf`).
New behavior introduced by the commit: if a bitswap block references is used by some root not fully deleted from the storage, it won't be deleted.
georgeee
force-pushed
the
sai/bitswaplip2phelperdefinitions
branch
from
7df326d
to
e979bdf
Compare
Update the test after reference tracking change.
Commentary from Sai: The 40 minute timeout wasn't enough for these changes with some runs. I found relatively low variance with a 60 minute timeout. I wasn't sure what the problem was initially on remote until i got the test passing locally after waiting longer. We run the new downloader test as well as the util test. It seems justified to me, because the downloader was not required before.
georgeee
force-pushed
the
sai/bitswaplip2phelperdefinitions
branch
from
e979bdf
to
637a9dc
Compare
georgeee
changed the title
georgeee
approved these changes
Oct 14, 2024
…phelperdefinitions
|
!ci-build-me |
1 similar comment
|
!ci-build-me |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.