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

feat(flags): [Breaking] Add cache superflag for alpha #7652

Merged
merged 5 commits into from
Apr 1, 2021

Conversation

rohanprasad
Copy link
Contributor

@rohanprasad rohanprasad commented Mar 25, 2021

This change introduces cache as a superflag, with size-mb and percentage as subflags for dgraph alphas.

Fixes DGRAPH-3212


This change is Reviewable

@rohanprasad rohanprasad changed the base branch from master to release/v21.03 March 25, 2021 16:57
dgraph/cmd/zero/run.go Outdated Show resolved Hide resolved
dgraph/cmd/alpha/run.go Show resolved Hide resolved
@rohanprasad rohanprasad changed the title feat(flags): Add cache superflag for alpha feat(flags): [Breaking] Add cache superflag for alpha Mar 26, 2021
@rohanprasad rohanprasad requested a review from NamanJain8 March 30, 2021 06:02
Copy link
Contributor

@NamanJain8 NamanJain8 left a comment

Choose a reason for hiding this comment

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

Looks good. Can you please verify if admin/config/cache_mb on alpha still works the same. It should work fine but should be good to confirm.

"Total size of cache (in MB) to be used in Dgraph.").
Flag("percentage",
"Cache percentages summing up to 100 for various caches (FORMAT: PostingListCache,"+
"PstoreBlockCache,PstoreIndexCache,WAL)").
Copy link
Contributor

@jarifibrahim jarifibrahim Mar 31, 2021

Choose a reason for hiding this comment

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

This needs to be changed. The pstoreblock cache and pstore index cache values are being fetched from the badger super flag. We don't need them over here.

Also, the wal is no longer supported.
@NamanJain8

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed the wal cache from flag. Also, we decided to keep the cache as it is.

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

Let's use Badger SuperFlag correctly in Dgraph.

Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @martinmr, @NamanJain8, @pawanrawal, @rohanprasad, and @vvbalaji-dgraph)

Copy link
Contributor

@NamanJain8 NamanJain8 left a comment

Choose a reason for hiding this comment

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

@manishrjain I will create a separate PR for badger flag changes.

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @martinmr, @NamanJain8, @pawanrawal, @rohanprasad, and @vvbalaji-dgraph)

@NamanJain8 NamanJain8 merged commit 32f1f58 into release/v21.03 Apr 1, 2021
@NamanJain8 NamanJain8 deleted the rohanprasad/add_cache_superflag branch April 1, 2021 17:22
aman-bansal pushed a commit that referenced this pull request Apr 8, 2021
This change introduces cache as a superflag, with size-mb and percentage as subflags for dgraph alphas.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants