Skip to content

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

Merged
NamanJain8 merged 5 commits intorelease/v21.03from
rohanprasad/add_cache_superflag
Apr 1, 2021
Merged

feat(flags): [Breaking] Add cache superflag for alpha#7652
NamanJain8 merged 5 commits intorelease/v21.03from
rohanprasad/add_cache_superflag

Conversation

@rohanprasad
Copy link
Copy Markdown
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
Comment thread dgraph/cmd/zero/run.go Outdated
Comment thread dgraph/cmd/alpha/run.go
@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
Copy Markdown
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.

Comment thread dgraph/cmd/alpha/run.go Outdated
"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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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