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(restore): add support for namespace aware restore #8968

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

shivaji-kharse
Copy link
Contributor

@shivaji-kharse shivaji-kharse commented Aug 23, 2023

This commit adds support for namespace aware restore. After this change, we can restore exactly one namespace from a backup of multiple namespaces into a dgraph cluster.

@dgraph-bot dgraph-bot added area/testing Testing related issues area/graphql Issues related to GraphQL support on Dgraph. area/core internal mechanisms go Pull requests that update Go code labels Aug 23, 2023
@shivaji-kharse shivaji-kharse marked this pull request as ready for review August 28, 2023 14:43
Copy link
Contributor

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

I don't see any code for moveTablet. How is this working?

dgraphtest/cluster.go Outdated Show resolved Hide resolved
dgraphtest/local_cluster.go Outdated Show resolved Hide resolved
dgraphtest/local_cluster.go Outdated Show resolved Hide resolved
graphql/admin/endpoints_ee.go Outdated Show resolved Hide resolved
graphql/admin/restore.go Outdated Show resolved Hide resolved
worker/online_restore.go Outdated Show resolved Hide resolved
worker/online_restore.go Outdated Show resolved Hide resolved
worker/restore_map.go Outdated Show resolved Hide resolved
worker/restore_map.go Outdated Show resolved Hide resolved
worker/restore_map.go Outdated Show resolved Hide resolved
@mangalaman93 mangalaman93 force-pushed the shiva/restore branch 2 times, most recently from 9b780b5 to 83f03f3 Compare August 30, 2023 13:40
@mangalaman93 mangalaman93 changed the title added namespace aware restore functionality feat(restore): add support for namespace aware restore Aug 30, 2023
Copy link
Contributor

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

More todos:

  1. I do not like the API, oneNamespaceOnly flag. we need to think more.
  2. Uprgade tests will fail because of the change in cluster.go restore function. It should support both old and new API
  3. We should test against a fresh cluster when zero doesn't have any tablets

dgraphtest/cluster.go Outdated Show resolved Hide resolved
worker/online_restore.go Outdated Show resolved Hide resolved
worker/restore_map.go Outdated Show resolved Hide resolved
worker/restore_map.go Outdated Show resolved Hide resolved
worker/restore_map.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

We should consider having a completely new API for this.

dgraphtest/cluster.go Outdated Show resolved Hide resolved
dgraphtest/cluster.go Outdated Show resolved Hide resolved
@shivaji-kharse shivaji-kharse force-pushed the shiva/restore branch 2 times, most recently from af6b376 to c10fcf0 Compare September 5, 2023 12:20
dgraphtest/cluster.go Show resolved Hide resolved
dgraphtest/cluster.go Outdated Show resolved Hide resolved
dgraphtest/cluster.go Outdated Show resolved Hide resolved
dgraphtest/cluster.go Outdated Show resolved Hide resolved
graphql/admin/endpoints_ee.go Show resolved Hide resolved
graphql/admin/restore.go Outdated Show resolved Hide resolved
graphql/admin/restore.go Outdated Show resolved Hide resolved
worker/online_restore.go Outdated Show resolved Hide resolved
worker/restore_map.go Show resolved Hide resolved
worker/restore_map.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

Looks pretty good now, only a few comments now.

dgraphtest/config.go Outdated Show resolved Hide resolved
graphql/admin/restore.go Show resolved Hide resolved
dgraphtest/cluster.go Outdated Show resolved Hide resolved
dgraphtest/cluster.go Outdated Show resolved Hide resolved
dgraphtest/cluster.go Outdated Show resolved Hide resolved
graphql/admin/endpoints_ee.go Show resolved Hide resolved
graphql/admin/restore.go Outdated Show resolved Hide resolved
worker/online_restore.go Outdated Show resolved Hide resolved
@mangalaman93 mangalaman93 merged commit e9f9b15 into main Sep 14, 2023
11 checks passed
@mangalaman93 mangalaman93 deleted the shiva/restore branch September 14, 2023 14:40
shivaji-kharse added a commit that referenced this pull request Mar 12, 2024
This commit adds support for namespace aware restore. After this change,
we can restore exactly one namespace from a backup of multiple
namespaces into the namespace 0 of a dgraph cluster.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core internal mechanisms area/graphql Issues related to GraphQL support on Dgraph. area/testing Testing related issues go Pull requests that update Go code
Development

Successfully merging this pull request may close these issues.

5 participants