-
Notifications
You must be signed in to change notification settings - Fork 66
Improvements to ClusterTables #3351
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
cc. @pcholakov for |
👍 makes sense to me, it was a PR suggestion that I should have rejected. |
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.
This looks really good. Thank you @AhmedSoliman for this PR. LGTM!
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.
Thanks a lot for updating our df tables @AhmedSoliman. The changes look good to me. +1 for merging :-)
/// Current known metadata version | ||
metadata_ver: DataType::UInt32, |
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.
Should we rename this into partition table version? I was for a second confused which metadata version this field refers to. On the other hand, it is in the partition table where it makes sense that this is the partition table version.
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'll follow through with the rest of the tables as well to make them consistent
This updates cluster tables in datafusion (restatectl sql): - Removes `node_state` as it doesn't align with the admin-less design. - Adds `state` in `node` table that uses gossip to show liveness - `partitions` table exposes v_current/v_next and replica-sets from gossip - Removes `persisted_lsn` since those tables are not meant to be backward compatible. ClusterTables are still internal/restatectl only
This updates cluster tables in datafusion (restatectl sql):
node_state
as it doesn't align with the admin-less design.state
innode
table that uses gossip to show livenesspartitions
table exposes v_current/v_next and replica-sets from gossippersisted_lsn
since those tables are not meant to be backward compatible. ClusterTables are still internal/restatectl only