sql/schemachanger: apply admission control to index merging #113705
Assignees
Labels
branch-release-23.2
Used to mark GA and release blockers and technical advisories for 23.2
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
GA-blocker
O-23.2-scale-testing
issues found during 23.2 scale testing
O-testcluster
Issues found or occurred on a test cluster, i.e. a long-running internal cluster
T-sql-foundations
SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
Comments
fqazi
added
C-bug
|
We should backport this to 23.1 and 22.2. |
|
should this be tagged as a 23.2 release blocker? |
fqazi
added a commit
to fqazi/cockroach
that referenced
this issue
Nov 2, 2023
Previously, when merging indexes as a part of our MVCC-compliant protocol, we did not properly flag the writes with the correct priority. As a result, on certain workloads, creating a secondary index could tank performance. To address this, this patch will set the priority for the writes during the merge phase of the backfill protocol. Fixes: cockroachdb#113705 Release note: Address performance regression that can happen when the declarative schema changer is being used for creating an index.
rafiss
added
GA-blocker
branch-release-23.2
craig bot
pushed a commit
that referenced
this issue
Nov 2, 2023
113713: sql/schemachanger: apply admission control to merge phase of backfill r=fqazi a=fqazi Previously, when merging indexes as a part of our MVCC-compliant protocol, we did not properly flag the writes with the correct priority. As a result, on certain workloads, creating a secondary index could tank performance. To address this, this patch will set the priority for the writes during the merge phase of the backfill protocol. Fixes: #113705 Release note (performance): Address performance regression that can happen when the declarative schema changer is being used to create an index with a concurrent workload. Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com>
blathers-crl bot
pushed a commit
that referenced
this issue
Nov 3, 2023
Previously, when merging indexes as a part of our MVCC-compliant protocol, we did not properly flag the writes with the correct priority. As a result, on certain workloads, creating a secondary index could tank performance. To address this, this patch will set the priority for the writes during the merge phase of the backfill protocol. Fixes: #113705 Release note (performance): Address performance regression that can happen when the declarative schema changer is being used to creating an index with a concurrent workload.
blathers-crl bot
pushed a commit
that referenced
this issue
Nov 3, 2023
Previously, when merging indexes as a part of our MVCC-compliant protocol, we did not properly flag the writes with the correct priority. As a result, on certain workloads, creating a secondary index could tank performance. To address this, this patch will set the priority for the writes during the merge phase of the backfill protocol. Fixes: #113705 Release note (performance): Address performance regression that can happen when the declarative schema changer is being used to creating an index with a concurrent workload.
williamkulju
added
O-testcluster
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Previously, admission control was only applied to the scan portion of the index merge operation, and the correct flag was never passed on the actual writes done by this operation. Update this code to pass the same flags as the scan.
The existing scan sets a priority flag:
cockroach/pkg/sql/backfill/mvcc_index_merger.go
Line 301 in 299f41e
The actual merges themselves don't:
https://github.com/cockroachdb/cockroach/blob/299f41ea88bdcf0d82295e8655dc7d948f38f57a/pkg/sql/backfill/mvcc_index_merger.go#L351C15-L351C15
In our scale testing we are seeing performance tank during the merge phase.
Jira issue: CRDB-33117
The text was updated successfully, but these errors were encountered: