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.

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

kvserver: add rangelog writes to a queue #105903

Closed
miraradeva opened this issue Jun 30, 2023 · 3 comments
Closed

kvserver: add rangelog writes to a queue #105903

miraradeva opened this issue Jun 30, 2023 · 3 comments
Assignees
Labels
branch-master Failures and bugs on the master branch. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-kv KV Team

Comments

@miraradeva
Copy link
Contributor

miraradeva commented Jun 30, 2023

We saw a scatter test with multiple rangelog timeouts in #104709. Bumping the timeout for the rangelog async task in #105898 helps but in the case of a scatter we may end up spawning too many async tasks that end up overwhelming the go scheduler. E.g. running a scatter on 100k ranges can result in 1 million rangelog events in a short amount of time.

Before #102813, writing to rangelog synchronously would act as a backpressure mechanism -- we wouldn't be able to go ahead with further conf changes until we'd flushed the write to the rangelog. Now we don't have that backpressure anymore, so we can pile up a huge backlog of writes.

One potential solution is to add all rangelog writes to a queue and then batch them.

Jira issue: CRDB-29264

@miraradeva miraradeva added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker T-kv KV Team labels Jun 30, 2023
@blathers-crl
Copy link

blathers-crl bot commented Jun 30, 2023

Hi @miraradeva, please add branch-* labels to identify which branch(es) this release-blocker affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@miraradeva miraradeva added the branch-master Failures and bugs on the master branch. label Jun 30, 2023
@nvanbenschoten
Copy link
Member

Before #102813, writing to rangelog synchronously would act as a backpressure mechanism -- we wouldn't be able to go ahead with further conf changes until we'd flushed the write to the rangelog. Now we don't have that backpressure anymore, so we can pile up a huge backlog of writes.

@miraradeva after #102813, we write to the rangelog non-atomically with respect to the replication change's txn, but we still log synchronously in the txn's commit trigger, right? Won't this still provide backpressure?

@miraradeva
Copy link
Contributor Author

Just seeing the above comment now, and I agree there shouldn't be a need for a separate queue for writing to the rangelog. We haven't seen any issues with it in the mean time either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-master Failures and bugs on the master branch. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-kv KV Team
Projects
None yet
Development

No branches or pull requests

2 participants