forked from cockroachdb/cockroach
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
storage: add "noop" intent resolution poisoning option
Manual testing in cockroachdb#15997 surfaced that one limiting factor in resolving many intents is contention on the transaction's abort cache entry. In one extreme test, I wrote 10E6 abortable intents into a single range, in which case the GC queue sends very large batches of intent resolution requests for the same transaction to the intent resolver. These requests all overlapped on the transaction's abort cache key, causing very slow progress, and ultimately preventing the GC queue from making a dent in the minute allotted to it. Generally this appears to be a somewhat atypical case, but since @nvanbenschoten observed something similar in cockroachdb#18199 it seemed well worth addressing, by means of 1. allow intent resolutions to not touch the abort span 2. correctly declare the keys for `ResolveIntent{,Range}` to only declare the abort cache key if it is actually going to be accessed. With these changes, the gc queue was able to clear out a million intents comfortably on my older 13" MacBook (single node). Also use this option in the intent resolver, where possible -- most transactions don't receive abort cache entries, and intents are often "found" by multiple conflicting writers. We want to avoid adding artificial contention there, though in many situations the same intent is resolved and so a conflict still exists. Migration: a new field number was added to the proto and the old one preserved. We continue to populate it. Downstream of Raft, we use the new field but if it's unset, synthesize from the deprecated field. I believe this is sufficient and we can just remove all traces of the old field in v1.3. (v1.1 uses the old, v1.2 uses the new with compatibility for the old, v1.3 only the new field).
- Loading branch information
Showing
8 changed files
with
633 additions
and
339 deletions.
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| // Copyright 2017 The Cockroach Authors. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or | ||
| // implied. See the License for the specific language governing | ||
| // permissions and limitations under the License. | ||
|
|
||
| package storage | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/roachpb" | ||
| "github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb" | ||
| "github.com/cockroachdb/cockroach/pkg/util/leaktest" | ||
| ) | ||
|
|
||
| func TestDeclareKeysResolveIntent(t *testing.T) { | ||
| defer leaktest.AfterTest(t)() | ||
|
|
||
| txnMeta := enginepb.TxnMeta{} | ||
| desc := roachpb.RangeDescriptor{ | ||
| RangeID: 99, | ||
| StartKey: roachpb.RKey("a"), | ||
| EndKey: roachpb.RKey("a"), | ||
| } | ||
| for _, test := range []struct { | ||
| status roachpb.TransactionStatus | ||
| poison roachpb.PoisonType | ||
| expSpans []string | ||
| }{ | ||
| { | ||
| status: roachpb.ABORTED, | ||
| poison: roachpb.PoisonType_Noop, | ||
| expSpans: []string{"1 0: {b-c}"}, | ||
| }, | ||
| { | ||
| status: roachpb.ABORTED, | ||
| poison: roachpb.PoisonType_Clear, | ||
| expSpans: []string{"1 0: {b-c}", `1 1: /{Local/RangeID/99/r/AbortCache/"00000000-0000-0000-0000-000000000000"-Min}`}, | ||
| }, | ||
| { | ||
| status: roachpb.ABORTED, | ||
| poison: roachpb.PoisonType_Do, | ||
| expSpans: []string{"1 0: {b-c}", `1 1: /{Local/RangeID/99/r/AbortCache/"00000000-0000-0000-0000-000000000000"-Min}`}, | ||
| }, | ||
| { | ||
| status: roachpb.COMMITTED, | ||
| poison: roachpb.PoisonType_Noop, | ||
| expSpans: []string{"1 0: {b-c}"}, | ||
| }, | ||
| { | ||
| status: roachpb.COMMITTED, | ||
| poison: roachpb.PoisonType_Clear, | ||
| expSpans: []string{"1 0: {b-c}", `1 1: /{Local/RangeID/99/r/AbortCache/"00000000-0000-0000-0000-000000000000"-Min}`}, | ||
| }, | ||
| { | ||
| status: roachpb.COMMITTED, | ||
| poison: roachpb.PoisonType_Do, | ||
| expSpans: []string{"1 0: {b-c}"}, | ||
| }, | ||
| } { | ||
| t.Run("", func(t *testing.T) { | ||
| ri := roachpb.ResolveIntentRequest{ | ||
| IntentTxn: txnMeta, | ||
| Status: test.status, | ||
| Poison: test.poison, | ||
| } | ||
| ri.Key = roachpb.Key("b") | ||
| ri.EndKey = roachpb.Key("c") | ||
|
|
||
| var spans SpanSet | ||
| var h roachpb.Header | ||
| h.RangeID = desc.RangeID | ||
| declareKeysResolveIntent(desc, h, &ri, &spans) | ||
| exp := strings.Join(test.expSpans, "\n") | ||
| if s := strings.TrimSpace(spans.String()); s != exp { | ||
| t.Errorf("expected %s, got %s", exp, s) | ||
| } | ||
| }) | ||
| } | ||
| } |
Oops, something went wrong.