-
Notifications
You must be signed in to change notification settings - Fork 4k
backfill: add a separate control for number of vectors merged per batch #155284
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
Add the 'bulkio.index_backfill.vector_merge_batch_size', which controls the number of rows to merge into vector indexes per transaction while the index is being created. This is analogous to the 'bulkio.index_backfill.merge_batch_size' setting, but it only applies when the target index is a vector index. By default, 3 vectors will be merged per transaction to reduce contention with fixup tasks. Fixes: cockroachdb#155283 Release note (sql change): Added the bulkio.index_backfill.vector_merge_batch_size cluster setting to control how many vectors to merge into a vector index per transaction during create operations. By default, this defaults to 3.
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
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.
@DrewKimball reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @fqazi)
bors r+ |
155192: backfill: split merge memory accounting mutex acquisitions r=mw5h a=mw5h Previously, constructMergeBatch() would get the mutex on memory accounting once per batch to minimize the number of atomics needed to perform memory accounting. With the addition of vector indexing, we now need to perform KV operations in order to construct batches, which can lead to deadlocks between KV and the memory accounting mutex. This patch splits the mutex acquisition into two acquisitions, releasing the mutex in between. Memory used is accumulated as merged entries are built and then accounting is notified for the entire batch. This has the effect of making the accounting lag behind allocation a bit, but that seems preferable than paying the cost of a mutex acquisition for every row merged. Fixes: #155190 Release note (bug fix): A potential deadlock during vector index creation has been corrected. 155284: backfill: add a separate control for number of vectors merged per batch r=mw5h a=mw5h Add the 'bulkio.index_backfill.vector_merge_batch_size', which controls the number of rows to merge into vector indexes per transaction while the index is being created. This is analogous to the 'bulkio.index_backfill.merge_batch_size' setting, but it only applies when the target index is a vector index. By default, 3 vectors will be merged per transaction to reduce contention with fixup tasks. Fixes: #155283 Release note (sql change): Added the bulkio.index_backfill.vector_merge_batch_size cluster setting to control how many vectors to merge into a vector index per transaction during create operations. By default, this defaults to 3. 155412: kvserver: introduce test harness for replica lifecycle events r=pav-kv a=arulajmani First two commits from #155408. This patch introduces the scaffolding for a harness to test various replica lifecycle events in a data-driven manner. The harness works from th context of a single node/store -- n1,s1. The intention is to print all engine interactions of the replica on this store on various replica lifecycle events. To kick things off, we add the ability to create a replica -- both initialized and uninitialized. Epic: none Release note: None Co-authored-by: Matt White <[email protected]> Co-authored-by: Arul Ajmani <[email protected]>
Build failed (retrying...):
|
Add the 'bulkio.index_backfill.vector_merge_batch_size', which controls the number of rows to merge into vector indexes per transaction while the index is being created. This is analogous to the 'bulkio.index_backfill.merge_batch_size' setting, but it only applies when the target index is a vector index. By default, 3 vectors will be merged per transaction to reduce contention with fixup tasks.
Fixes: #155283
Release note (sql change): Added the
bulkio.index_backfill.vector_merge_batch_size cluster setting to control how many vectors to merge into a vector index per transaction during create operations. By default, this defaults to 3.