-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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(schema): do schema versioning and make backup non-blocking for indexing #7852
Conversation
non-blocking for indexing
8edc558
to
e683f87
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got some comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do test for predicate moves and snapshots, etc.
Reviewed 13 of 13 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @NamanJain8 and @vvbalaji-dgraph)
worker/draft.go, line 230 at r1 (raw file):
case opIndexing: for otherId, otherCloser := range n.ops { if otherId == opBackup {
Ideally, we should look at the backup timestamp. If indexing has a higher ts, then it's OK. If not, then not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dismissed @jarifibrahim and @manishrjain from 2 discussions.
Reviewable status: 10 of 13 files reviewed, all discussions resolved (waiting on @manishrjain and @vvbalaji-dgraph)
worker/draft.go, line 230 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
Ideally, we should look at the backup timestamp. If indexing has a higher ts, then it's OK. If not, then not.
Done.
worker/draft.go, line 626 at r1 (raw file):
Previously, jarifibrahim (Ibrahim Jarif) wrote…
The startTs is actually the
MaxAssigned
. See line 605. You might end up writing data at the wrong timestamp.
We should use the same timestamp as the one used for index rebuild (MaxAssigned
). Max assigned is used here to index all the data that has been committed up until this point.
…ndexing (#7852) Issue: We used to write schema/types at timestamp=1. That single bit violates the Snapshot Isolation. Hence, we couldn't run schema updates and other operations like backup concurrently. This is an issue in a multi-tenant cluster because if the backup is running we couldn't update the schema. Fix: This PR writes the schema/types at the startTs of the transaction and enables the schema updates to be made even if the backup is running. (cherry picked from commit f376a40)
…g for i… (#7856) * feat(schema): do schema versioning and make backup non-blocking for indexing (#7852) Issue: We used to write schema/types at timestamp=1. That single bit violates the Snapshot Isolation. Hence, we couldn't run schema updates and other operations like backup concurrently. This is an issue in a multi-tenant cluster because if the backup is running we couldn't update the schema. Fix: This PR writes the schema/types at the startTs of the transaction and enables the schema updates to be made even if the backup is running. (cherry picked from commit f376a40) * fix the predicate move
… for i… (#7856) * feat(schema): do schema versioning and make backup non-blocking for indexing (#7852) Issue: We used to write schema/types at timestamp=1. That single bit violates the Snapshot Isolation. Hence, we couldn't run schema updates and other operations like backup concurrently. This is an issue in a multi-tenant cluster because if the backup is running we couldn't update the schema. Fix: This PR writes the schema/types at the startTs of the transaction and enables the schema updates to be made even if the backup is running. (cherry picked from commit f376a40) * fix the predicate move (cherry picked from commit eb0a04b)
… for i… (#7856) * feat(schema): do schema versioning and make backup non-blocking for indexing (#7852) Issue: We used to write schema/types at timestamp=1. That single bit violates the Snapshot Isolation. Hence, we couldn't run schema updates and other operations like backup concurrently. This is an issue in a multi-tenant cluster because if the backup is running we couldn't update the schema. Fix: This PR writes the schema/types at the startTs of the transaction and enables the schema updates to be made even if the backup is running. (cherry picked from commit f376a40) * fix the predicate move (cherry picked from commit eb0a04b)
#7856) (#7873) * feat(schema): do schema versioning and make backup non-blocking for indexing (#7852) Issue: We used to write schema/types at timestamp=1. That single bit violates the Snapshot Isolation. Hence, we couldn't run schema updates and other operations like backup concurrently. This is an issue in a multi-tenant cluster because if the backup is running we couldn't update the schema. Fix: This PR writes the schema/types at the startTs of the transaction and enables the schema updates to be made even if the backup is running. (cherry picked from commit f376a40) * fix the predicate move (cherry picked from commit eb0a04b)
Issue:
We used to write schema/types at
timestamp=1
. That single bit violates the Snapshot Isolation. Hence, we couldn't run schema updates and other operations like backup concurrently. This is an issue in a multi-tenant cluster because if the backup is running we couldn't update the schema.Fix:
This PR writes the schema/types at the
startTs
of the transaction and enables the schema updates to be made even if the backup is running.This change is