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

Sloop was not able to reduce the size on disk after GC runs. This also plays a big role in memory consumption. #114

Merged
merged 5 commits into from
Apr 24, 2020

Conversation

sana-jawad
Copy link
Collaborator

PR includes following changes:

  1. Upgraded Badger version.
  2. Added flattening at start up time.
  3. Fixed the event count spreading issue which resulted in uneven data distribution across partitions.
  4. Moved to drop prefix as it yields better space claim.
  5. Added feature flag for switching to delete prefix. Also changed the numberfversions to 0 so that delete prefix would reclaim space. GC doesn't work? (not cleaning up SST files properly) hypermodeinc/badger#1228
  6. Fixed the issue of unclaimed !badger!move prefixes which are never cleaned up. Details: Does dropPrefix remove !badger!move keys? hypermodeinc/badger#1288
  7. Added support in debugging pages to see internal keys.

Drop Prefix VS Delete Keys
In first 12 hours GC was run using Drop Prefix and rest of 12 hours were run using Delete Prefix
image

Debug Page showing internal keys info:

image

Copy link
Contributor

@duke-harlan duke-harlan left a comment

Choose a reason for hiding this comment

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

passing on a few comments - still reviewing (sorry, keep getting distracted)

pkg/sloop/common/db_utilities.go Outdated Show resolved Hide resolved
pkg/sloop/common/db_utilities.go Show resolved Hide resolved
pkg/sloop/processing/eventcount.go Outdated Show resolved Hide resolved
pkg/sloop/processing/eventcount.go Outdated Show resolved Hide resolved
Copy link
Contributor

@duke-harlan duke-harlan left a comment

Choose a reason for hiding this comment

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

some more comments - still trying to grok all of the delete key / drop prefix logic

pkg/sloop/server/server.go Outdated Show resolved Hide resolved
pkg/sloop/storemanager/storemanager.go Outdated Show resolved Hide resolved
pkg/sloop/storemanager/storemanager.go Show resolved Hide resolved
pkg/sloop/webserver/debug.go Outdated Show resolved Hide resolved
pkg/sloop/webserver/debug.go Outdated Show resolved Hide resolved
pkg/sloop/webserver/debug.go Outdated Show resolved Hide resolved
pkg/sloop/webserver/debug.go Outdated Show resolved Hide resolved
Copy link
Contributor

@duke-harlan duke-harlan left a comment

Choose a reason for hiding this comment

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

more comments

pkg/sloop/processing/eventcount.go Outdated Show resolved Hide resolved
pkg/sloop/processing/eventcount.go Outdated Show resolved Hide resolved
pkg/sloop/storemanager/storemanager.go Outdated Show resolved Hide resolved
pkg/sloop/common/db_utilities.go Show resolved Hide resolved
pkg/sloop/webserver/debug.go Show resolved Hide resolved
pkg/sloop/storemanager/storemanager.go Outdated Show resolved Hide resolved
pkg/sloop/storemanager/storemanager.go Show resolved Hide resolved
@mallow111
Copy link
Collaborator

left a few comments

mallow111
mallow111 previously approved these changes Apr 24, 2020
Copy link
Collaborator

@mallow111 mallow111 left a comment

Choose a reason for hiding this comment

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

lgtm, make sure address comments from @duke-harlan, if there is a separate PR needed, please create the PR or issue so that we can track of it, thank you!

@sana-jawad sana-jawad merged commit c595202 into salesforce:master Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants