Make repo gc call CollectGarbage on datastore#4578
Conversation
ghost
assigned
|
|
||
| err = gds.CollectGarbage() | ||
| if err != nil { | ||
| output <- Result{Error: err} |
There was a problem hiding this comment.
This can deadlock if the context has been canceled and output is full. However, we already have this bug (above) so feel free to fix it separately. See #4593.
repo/fsrepo/datastores.go
Outdated
|
|
||
| vls, ok := params["vlogFileSize"] | ||
| if !ok { | ||
| c.vlogFileSize = 1 << 30 |
There was a problem hiding this comment.
Why? Could you add a comment explaining this.
There was a problem hiding this comment.
It's the default size, will add a comment
magik6k
force-pushed
the
feat/datastore-gc
branch
3 times, most recently
from
8c25e64 to
0e58b0c
Compare
|
This is currently blocked by go-datastore being updated in go-ds-measure and not here: ipfs/go-ds-measure@d287c4e#diff-b9cfc7f2cdf78a7f4b91a753d10865a2 |
|
Blocked on #4610. |
|
Unblocked, needs rebase. |
magik6k
force-pushed
the
feat/datastore-gc
branch
from
0e58b0c to
2da5b67
Compare
|
Rebased |
Stebalien
left a comment
Stebalien
left a comment
There was a problem hiding this comment.
LGTM. Eventually, the we should probably expose this through the blockstore (as GC is really happening at the blockstore level) but we can do that later.
There was a problem hiding this comment.
What is the new vlogFileSize parameter and how does it relate to calling CollectGarabge on the datastores?
If this is not related to the purpose of this P.R. could we separate it out into a separate commit?
If it is related could we mention it in the commit message so someone not familiar with the Badger Datastore code knows how it relates?
magik6k
force-pushed
the
feat/datastore-gc
branch
from
2da5b67 to
a8ccc6a
Compare
Badger is currently the only DS that implements GC, the minimum reclaimable space is defined as
Done |
|
@magik6k sorry, need a rebase here. |
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
magik6k
force-pushed
the
feat/datastore-gc
branch
from
a8ccc6a to
d7aa2f7
Compare
4 participants
Depends on ipfs/go-ds-measure#13
I'm not sure how to approach adding tests for this. Badger does implement this interface and it does get called, though due to it's GC weirdness I'm unable to come up with scenario in which it will actually free-up some space.
(Badger was updated to current master, doesn't seem to break anything from my testing)