-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
batcheval: keep correct stats in AddSSTable without overlap #36907
Conversation
Clarify why it's OK to ingest non-atomically with applying the Raft command. Release note: None
Release note: None
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.
Though should note that the assertion that IMPORT SSTs don't overlap is only true of distsql sorted import -- direct import will probably be the default in 19.2 and it will likely make overlapping SSTs.
pkg/storage/replica_raft.go
Outdated
// | ||
// Note that in the event of an ill-timed crash, the data may have been | ||
// ingested (i.e. become "visible") without the Raft command having | ||
// marked as applied. However, the node will only serve reads once a new |
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.
Is this true with follower reads?
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.
Follower reads will see extra data, which is weird (and the data could be out-of-order with the raft log...) anyway, hard to figure what exactly a problem here would be, but closed timestamps would definitely go down the drain and everything is pretty undefined with sst ingestion anyway. I rearranged the comment and clarified.
For distributed index backfill, we recently switched to allowing the stats of the data in the SST to be blindly added to that of the range (thus introducing ContainsEstimates=true). However, if the keyspace being ingested into is empty (such as usually the case during IMPORT/RESTORE), it seems a little silly to make all the consistency checks for the next 24h twice as expensive. Probe whether the keyspace is empty and if so, don't pretend to introduce an approximation - the stats in this case are exact. Release note: None
Release note: None
Superseded by many changes in cmd_add_sstable.go that have landed since. |
For distributed index backfill, we recently switched to allowing the stats
of the data in the SST to be blindly added to that of the range
(thus introducing ContainsEstimates=true). However, if the keyspace being
ingested into is empty (such as usually the case during IMPORT/RESTORE), it
seems a little silly to make all the consistency checks for the next 24h
twice as expensive. Probe whether the keyspace is empty and if so, don't
pretend to introduce an approximation - the stats in this case are exact.
Release note: None