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

Periodically garbage collect #51

Closed
Stebalien opened this issue Mar 8, 2019 · 8 comments
Closed

Periodically garbage collect #51

Stebalien opened this issue Mar 8, 2019 · 8 comments
Labels
help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature

Comments

@Stebalien
Copy link
Member

We should periodically garbage collect in case the user has garbage collection turned off. Otherwise, we'll never delete anything.

@magik6k magik6k added kind/enhancement A net-new feature or improvement to an existing feature help wanted Seeking public contribution on this issue labels May 16, 2019
@raulk
Copy link
Member

raulk commented Jul 4, 2019

Why is gcDiscardRatio statically configured to 0.1? The struct field is unexported :-(

@Stebalien
Copy link
Member Author

Not exporting it was probably a mistake, @magik6k?

@raulk
Copy link
Member

raulk commented Jul 4, 2019

Gotcha, I'll submit a PR.

Another thing, overriding the error here creates ambiguity: https://github.com/ipfs/go-ds-badger/blob/master/datastore.go#L286.

Badger returns nil if one file was collected, and callers may use this as a hint to continue collecting by calling GC again.

Overriding the "nothing was collected" error to nil makes it impossible to discriminate whether it's worth calling GC again or not :-(

If we want to decouple this from badger implementation-specific errors, we can return a second value to signal if something was collected, or not, or we don't know (some stores may not expose this event).

@Stebalien
Copy link
Member Author

If we want to decouple this from badger implementation-specific errors, we can return a second value to signal if something was collected, or not, or we don't know (some stores may not expose this event).

That's the issue. However, for every datastore except badger, re-running GC till it removes nothing is just a waste of time so I'm not sure how useful that is.

See #56 that just does this automatically.

@magik6k
Copy link
Member

magik6k commented Jul 5, 2019

Not exporting it was probably a mistake, @magik6k?

Yeah, I probably meant to export it, but also set some value that would mostly work to quickly test and never changed it

@aarshkshah1992
Copy link
Contributor

@Stebalien Is this up for grabs ?

@Stebalien
Copy link
Member Author

Yes! (it's also pretty easy to review so it should get merged pretty quickly).

@Stebalien
Copy link
Member Author

Fixed in #72.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants