-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
ttljob: paginate ranges when doing TTL #76726
Conversation
c85a8ad
to
d16ada0
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.
lgtm! had some comments for future and a nit about the setting description
i'm not very familiar with this part of the KV api (like RangeMetaKey or MustAddr), so if needed, feel free to reach out to them for another review
@@ -317,7 +317,11 @@ func TestRowLevelTTLJobRandomEntries(t *testing.T) { | |||
}) | |||
defer cleanupFunc() | |||
|
|||
rangeBatchSize := 1 + rng.Intn(3) |
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.
if the default is 100, should the test be closer to that?
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.
we used a smaller number here here because we have less ranges (maximum of ~8).
@@ -317,7 +317,11 @@ func TestRowLevelTTLJobRandomEntries(t *testing.T) { | |||
}) | |||
defer cleanupFunc() | |||
|
|||
rangeBatchSize := 1 + rng.Intn(3) | |||
t.Logf("range batch size: %d", rangeBatchSize) |
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.
random thought - we should have one place to randomize + log all the TTL settings
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.
there is more or less this above -- it's just this is the only one randomised by a cluster setting
bors r=rafiss thanks! |
Canceled. |
bors r=rafiss |
76726: ttljob: paginate ranges when doing TTL r=rafiss a=otan This commit batches ranges instead of scanning them in all at once. The range batch size is controlled by the cluster setting `sql.ttl.range_batch_size`. I don't anticipate this needing to be controlled per table. Release note: None Co-authored-by: Oliver Tan <[email protected]>
Build failed: |
This commit batches ranges instead of scanning them in all at once. The range batch size is controlled by the cluster setting `sql.ttl.range_batch_size`. I don't anticipate this needing to be controlled per table. Release note: None
bors r=rafiss |
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
Build succeeded: |
This commit batches ranges instead of scanning them in all at once. The
range batch size is controlled by the cluster setting
sql.ttl.range_batch_size
. I don't anticipate this needing to becontrolled per table.
Release note: None