-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29207 The backup system table should be considered a system table #6842
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
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| } else if (Bytes.equals(NamespaceDescriptor.BACKUP_NAMESPACE_NAME, namespace)) { | ||
| this.namespace = NamespaceDescriptor.BACKUP_NAMESPACE_NAME; |
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.
What if user has configured a custom table name as in https://github.com/apache/hbase/blob/master/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java#L1194 ?
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.
I’m not sure, to be honest. If we were back in time I’d recommend not supporting that feature at all. Do you have any ideas?
709e203 to
8f08296
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Wow, I see now why this wasn’t listed as a system table initially. We take snapshots of the backups table periodically, and you cannot take snapshots of system tables. Test failures caught this disconnect of intentions. I’ll think a bit about this, but I wonder if we should make an exception for allowing backup system table snapshots. |
8f08296 to
2eaa884
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Wait you can't take a snapshot of meta? I feel like this is something I've done before. Maybe I was just taking a "dump"/export... |
|
According to the precondition tripped by those tests, one cannot |
ndimiduk
left a comment
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.
Some might argue that this table should be named system:backup instead of backup:system, but c'est la vie
Yes this is quite unfortunate.
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
…le (#6842) Co-authored-by: Ray Mattingly <[email protected]> Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Nihal Jain <[email protected]>
…le (#6842) Co-authored-by: Ray Mattingly <[email protected]> Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Nihal Jain <[email protected]>
…le (#6842) Co-authored-by: Ray Mattingly <[email protected]> Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Nihal Jain <[email protected]>
…le (#6842) (#6928) Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Co-authored-by: Ray Mattingly <[email protected]>
…le (#6842) (#6929) Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Co-authored-by: Ray Mattingly <[email protected]>
…le (#6842) (#6930) Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Co-authored-by: Ray Mattingly <[email protected]>
…le (apache#6842) (apache#6929) Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Co-authored-by: Ray Mattingly <[email protected]>
Some might argue that this table should be named system:backup instead of backup:system, but c'est la vie