Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Set ttl_delete_rate_limit to 100 instead of current 0 (unlimited) #122698

Closed
dikshant opened this issue Apr 19, 2024 · 1 comment · Fixed by #124184
Closed

Set ttl_delete_rate_limit to 100 instead of current 0 (unlimited) #122698

dikshant opened this issue Apr 19, 2024 · 1 comment · Fixed by #124184
Assignees
@Dedej-Bergin
Labels
A-row-level-ttl C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
SQL Foundations

Comments

@dikshant
Copy link

dikshant commented Apr 19, 2024 •
edited by exalate-issue-sync bot

Currently the delete rate limit is set to 0. This can cause adverse impact to foreground latency. A quick survey of our customer indicates most have a value of around 100. In any case a bounded value would be better than the current unlimited.

We should set this for only newly TTL enabled tables. Existing tables that have ttl should have their defaults be at 0 to prevent confusions around "why is my TTL job slow after upgrade?"

Jira issue: CRDB-38030

Epic CRDB-18322
@dikshant dikshant added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) A-row-level-ttl labels Apr 19, 2024
@blathers-crl blathers-crl bot added this to Triage in SQL Foundations Apr 19, 2024
@rafiss
Copy link
Collaborator

rafiss commented Apr 23, 2024

There are plans to integrate TTL with disk I/O limiting: #121782

Maybe we can just change the default on older branches, and leave master at 0 for right now. That way, it will be easier to see if disk AC is having the desired effects on master. We can decide when 24.2 is being cut if we want it to also have a limit of 100.

@Dedej-Bergin Dedej-Bergin mentioned this issue May 14, 2024
Merged
cockroach-dev-inf pushed a commit that referenced this issue May 14, 2024
@Dedej-Bergin
ttl/ttlbase: Set ttl_delete_rate_limit to 100 instead of current 0 (u…
25d1723 
…nlimited)

Previously we were using no rate limiting by default, and this could cause a
slowdown. By changing the default rate limit to 100, we will avoid many possible
slowdows caused by having no rate limiter.

Fixes: #122698

Release note (performance improvement): We are now setting ttl_delete_rate_limit to 100,
this sets the default rate limit for deleting expired rows to 100.
Dedej-Bergin added a commit to Dedej-Bergin/cockroach that referenced this issue May 14, 2024
@Dedej-Bergin
ttl/ttlbase: Set ttl_delete_rate_limit to 100 instead of current 0
fc32030 
Previously we were using no rate limiting by default, and this could cause a
slowdown. By changing the default rate limit to 100, we will avoid many possible
slowdows caused by having no rate limiter.

Fixes: cockroachdb#122698

Release note (performance improvement): We are now setting ttl_delete_rate_limit to 100,
this sets the default rate limit for deleting expired rows to 100.
Dedej-Bergin added a commit to Dedej-Bergin/cockroach that referenced this issue May 15, 2024
@Dedej-Bergin
ttl/ttlbase: Set ttl_delete_rate_limit to 100 instead of current 0
3bc04bb 
Previously we were using no rate limiting by default, and this could cause a
slowdown. By changing the default rate limit to 100, we will avoid many possible
slowdows caused by having no rate limiter.

Fixes: cockroachdb#122698

Release note (performance improvement): We are now setting ttl_delete_rate_limit to 100,
this sets the default rate limit for deleting expired rows to 100.
craig bot pushed a commit that referenced this issue May 17, 2024
@Dedej-Bergin
Merge #124184
8fb653b 
124184: ttl/ttlbase: Set ttl_delete_rate_limit to 100 instead of current 0 r=Dedej-Bergin a=Dedej-Bergin

Previously we were using no rate limiting by default, and this could cause a slowdown. By changing the default rate limit to 100, we will avoid many possible slowdows caused by having no rate limiter.

Fixes: #122698

Release note (sql change): We are now setting ttl_delete_rate_limit to 100, this sets the default rate limit for deleting expired rows to 100.

Co-authored-by: Bergin Dedej <bergin.dedej@cockroachlabs.com>
@craig craig bot closed this as completed in 87acf81 May 17, 2024
SQL Foundations automation moved this from Triage to Done May 17, 2024
blathers-crl bot pushed a commit that referenced this issue May 17, 2024
@Dedej-Bergin
ttl/ttlbase: Set ttl_delete_rate_limit to 100 instead of current 0
f48b9b3 
Previously we were using no rate limiting by default, and this could cause a
slowdown. By changing the default rate limit to 100, we will avoid many possible
slowdows caused by having no rate limiter.

Fixes: #122698

Release note (sql change): We are now setting ttl_delete_rate_limit to 100,
this sets the default rate limit for deleting expired rows to 100.
@blathers-crl blathers-crl bot mentioned this issue May 17, 2024
Merged
blathers-crl bot pushed a commit that referenced this issue May 17, 2024
@Dedej-Bergin
ttl/ttlbase: Set ttl_delete_rate_limit to 100 instead of current 0
ae49749 
Previously we were using no rate limiting by default, and this could cause a
slowdown. By changing the default rate limit to 100, we will avoid many possible
slowdows caused by having no rate limiter.

Fixes: #122698

Release note (sql change): We are now setting ttl_delete_rate_limit to 100,
this sets the default rate limit for deleting expired rows to 100.
@blathers-crl blathers-crl bot mentioned this issue May 17, 2024
Merged
@Dedej-Bergin Dedej-Bergin mentioned this issue May 17, 2024
Merged
cockroach-dev-inf pushed a commit that referenced this issue May 17, 2024
@Dedej-Bergin
ttl/ttlbase: Set ttl_delete_rate_limit to 100 instead of current 0
0302fbc 
Previously we were using no rate limiting by default, and this could cause a
slowdown. By changing the default rate limit to 100, we will avoid many possible
slowdows caused by having no rate limiter.

Fixes: #122698

Release note (sql change): We are now setting ttl_delete_rate_limit to 100,
this sets the default rate limit for deleting expired rows to 100.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Dedej-Bergin Dedej-Bergin

Labels
A-row-level-ttl C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
SQL Foundations
  
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ttl/ttlbase: Set ttl_delete_rate_limit to 100 instead of current 0 Dedej-Bergin/cockroach
3 participants
@rafiss @dikshant @Dedej-Bergin