Skip to content

feat(multi-tenancy): make drop data namespace aware #7789

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

Merged
merged 5 commits into from
May 6, 2021

Conversation

NamanJain8
Copy link
Contributor

@NamanJain8 NamanJain8 commented May 6, 2021

This PR makes DropData namespace-aware in a multi-tenant system. Now, guardians of a namespace can call drop data on their namespace. Earlier only the guardian of the galaxy was allowed to do cluster-wide drop operation.


This change is Reviewable

@NamanJain8 NamanJain8 marked this pull request as draft May 6, 2021 11:47
@NamanJain8 NamanJain8 marked this pull request as ready for review May 6, 2021 13:15
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.

:lgtm:

Reviewed 10 of 10 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @NamanJain8 and @vvbalaji-dgraph)


edgraph/access.go, line 49 at r1 (raw file):

}

func upsertGuardianAndGroot(closer *z.Closer, ns uint64) {

// do nothing.


worker/cdc_ee.go, line 123 at r1 (raw file):

	for ts, events := range cdc.pendingTxnEvents {
		if len(events) > 0 && binary.BigEndian.Uint64(events[0].Meta.Namespace) == ns {
			delete(cdc.pendingTxnEvents, ts)

Don't need to delete any events before drop all.


worker/cdc_ee.go, line 266 at r1 (raw file):

						return
					}
					cdc.resetPendingEventsForNs(ns)

no need.


worker/draft.go, line 329 at r1 (raw file):

		// Clear entire cache.
		// TODO(Naman): Would it be okay for now to reset cluster wide cache on drop data.

We don't want to drop entire clusters cache, just due to one namespace.

Copy link
Contributor Author

@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.

Dismissed @aman-bansal and @manishrjain from 3 discussions.
Reviewable status: 7 of 10 files reviewed, 2 unresolved discussions (waiting on @aman-bansal, @manishrjain, and @vvbalaji-dgraph)


edgraph/access.go, line 49 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

// do nothing.

Done.


worker/cdc_ee.go, line 123 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Don't need to delete any events before drop all.

As discussed with @aman-bansal , these events are cleared on the commit of the transaction. Hence, we need to clear out all these events, else they would stay in memory.


worker/cdc_ee.go, line 266 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

no need.

Replied above.


worker/cdc_ee.go, line 433 at r1 (raw file):

Previously, aman-bansal (aman bansal) wrote…

please don't panic

Done.


worker/draft.go, line 329 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

We don't want to drop entire clusters cache, just due to one namespace.

Done.

@NamanJain8 NamanJain8 merged commit 260a789 into release/v21.03-slash May 6, 2021
@NamanJain8 NamanJain8 deleted the naman/drop-data branch May 6, 2021 18:37
NamanJain8 added a commit that referenced this pull request May 7, 2021
This PR makes DropData namespace-aware in a multi-tenant system. Now, guardians of a namespace can call drop data on their namespace. Earlier only the guardian of the galaxy was allowed to do cluster-wide drop operation.

(cherry picked from commit 260a789)
NamanJain8 added a commit that referenced this pull request May 10, 2021
This PR makes DropData namespace-aware in a multi-tenant system. Now, guardians of a namespace can call drop data on their namespace. Earlier only the guardian of the galaxy was allowed to do cluster-wide drop operation.

(cherry picked from commit 260a789)
Rajakavitha1 added a commit to Rajakavitha1/dgraph-docs that referenced this pull request Sep 9, 2021
- updated the documentation based on hypermodeinc/dgraph#7789
danielmai pushed a commit to dgraph-io/dgraph-docs that referenced this pull request Oct 1, 2021
Updated the documentation based on hypermodeinc/dgraph#7789
MichelDiz pushed a commit to dgraph-io/dgraph-docs that referenced this pull request Dec 15, 2022
MichelDiz added a commit to dgraph-io/dgraph-docs that referenced this pull request Apr 7, 2023
Updated the documentation based on
hypermodeinc/dgraph#7789
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.

3 participants