Skip to content

Commit 3fc29ad

Browse files
nvbnvb
committed
kv: resolve conflicting intents immediately for latency-sensitive requests
Fixes #50390. Related to #60585. Related to #103126. Closes #64500, which was an earlier attempt to solve this issue using a similar approach. This commit addresses the comments on that PR, which focused on the pagination of intent resolution when bypassing the request batcher. We still don't try to solve this issue, and instead limit the cases where intent resolution bypasses the request batcher to those where pagination is not necessary. This commit adds a new `sendImmediately` option to the `IntentResolver` `ResolveOptions`, which instructs the `IntentResolver` to send the provided intent resolution requests immediately, instead of adding them to a batch and waiting up to 10ms (defaultIntentResolutionBatchWait) for that batch to fill up with other intent resolution requests. This can be used to avoid any batching-induced latency and should be used only by foreground traffic that is willing to trade off some throughput for lower latency. The commit then sets this flag for single-key intent resolution requests initiated by the `lockTableWaiter`. Unlike most calls into the `IntentResolver`, which are performed by background tasks that are more than happy to trade increased latency for increased throughput, the `lockTableWaiter` calls into the `IntentResolver` on behalf of a foreground operation. It wants intent resolution to complete as soon as possible, so it is willing to trade reduced throughput for reduced latency. I tested this out by writing 10,000 different intents in a normal-priority transaction and then scanning over the table using a high-priority transaction. The test was performed on a 3-node roachprod cluster to demonstrate the effect with realistic network and disk latencies. ``` -- session 1 CREATE TABLE keys (k BIGINT NOT NULL PRIMARY KEY); BEGIN; INSERT INTO keys SELECT generate_series(1, 10000); -- session 2 BEGIN PRIORITY HIGH; SELECT count(*) FROM keys; ``` Without this change, the scan took 70.078s. With this change, the scan took 15.958s. This 78% speed up checks out. Each encountered intent is resolved serially (see #103126), so the per-intent latency drops from 7ms to 1.6ms. This improvement by about 5ms agrees with the `defaultIntentResolutionBatchIdle`, which delays each resolution request that passes through a RequestBatcher. With this change, these resolve intent requests are issued immediately and this delay is not experienced. While this is a significant improvement and will be important for Read Committed transactions after #102014, this is still not quite enough to resolve #103126. For that, we need to batch the resolve intent requests themselves using a form of deferred intent resolution like we added in #49218 (for finalized transactions). A similar improvement is seen for scans that encounter many abandoned intents from many different transactions. This scenario bypasses the deferred intent resolution path added in #49218, because the intents are each written by different transactions. The speedup for this scenario was presented in #64500. Release note (performance improvement): SQL statements that must clean up intents from many different previously abandoned transactions now do so moderately more efficiently.
1 parent 11c3c83 commit 3fc29ad

1 file changed

Lines changed: 73 additions & 12 deletions

File tree

‎pkg/kv/kvserver/intentresolver/intent_resolver.go‎

Lines changed: 73 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,23 @@ type ResolveOptions struct {
845845
// The original transaction timestamp from the earliest txn epoch; if
846846
// supplied, resolution of intent ranges can be optimized in some cases.
847847
MinTimestamp hlc.Timestamp
848+
// If set, instructs the IntentResolver to send the intent resolution requests
849+
// immediately, instead of adding them to a batch and waiting for that batch
850+
// to fill up with other intent resolution requests. This can be used to avoid
851+
// any batching-induced latency, and should be used only by foreground traffic
852+
// that is willing to trade off some throughput for lower latency.
853+
//
854+
// In addition to disabling batching, the option will also disable key count
855+
// and byte size pagination. All requests will be sent in the same batch
856+
// (subject to splitting on range boundaries) and no MaxSpanRequestKeys or
857+
// TargetBytes limits will be assigned to limit the number or size of intents
858+
// resolved by multi-point or ranged intent resolution. Users of the flag
859+
// should be conscious of this.
860+
//
861+
// Because of these limitations, the flag is kept internal to this package. If
862+
// we want to expose the flag and use it in more cases, we will first need to
863+
// support key count and byte size pagination when bypassing the batcher.
864+
sendImmediately bool
848865
}
849866

850867
// lookupRangeID maps a key to a RangeID for best effort batching of intent
@@ -873,45 +890,79 @@ type lockUpdates interface {
873890
Index(i int) roachpb.LockUpdate
874891
}
875892

893+
var _ lockUpdates = (*txnLockUpdates)(nil)
894+
var _ lockUpdates = (*singleLockUpdate)(nil)
895+
var _ lockUpdates = (*sliceLockUpdates)(nil)
896+
876897
type txnLockUpdates roachpb.Transaction
877898

878-
// Len returns the number of LockSpans in a txnLockUpdates,
879-
// as part of the lockUpdates interface implementation.
899+
// Len implements the lockUpdates interface.
880900
func (t *txnLockUpdates) Len() int {
881901
return len(t.LockSpans)
882902
}
883903

884-
// Index produces a LockUpdate from the respective LockSpan, when called on
885-
// txnLockUpdates. txnLockUpdates implements the lockUpdates interface.
904+
// Index implements the lockUpdates interface.
886905
func (t *txnLockUpdates) Index(i int) roachpb.LockUpdate {
887906
return roachpb.MakeLockUpdate((*roachpb.Transaction)(t), t.LockSpans[i])
888907
}
889908

909+
type singleLockUpdate roachpb.LockUpdate
910+
911+
// Len implements the lockUpdates interface.
912+
func (s *singleLockUpdate) Len() int {
913+
return 1
914+
}
915+
916+
// Index implements the lockUpdates interface.
917+
func (s *singleLockUpdate) Index(i int) roachpb.LockUpdate {
918+
if i != 0 {
919+
panic("index out of bounds")
920+
}
921+
return roachpb.LockUpdate(*s)
922+
}
923+
890924
type sliceLockUpdates []roachpb.LockUpdate
891925

892-
// Len returns the number of LockUpdates in sliceLockUpdates,
893-
// as part of the lockUpdates interface implementation.
926+
// Len implements the lockUpdates interface.
894927
func (s *sliceLockUpdates) Len() int {
895928
return len(*s)
896929
}
897930

898-
// Index trivially produces a LockUpdate when called on sliceLockUpdates.
899-
// sliceLockUpdates implements the lockUpdates interface.
931+
// Index implements the lockUpdates interface.
900932
func (s *sliceLockUpdates) Index(i int) roachpb.LockUpdate {
901933
return (*s)[i]
902934
}
903935

904-
// ResolveIntent synchronously resolves an intent according to opts.
936+
// ResolveIntent synchronously resolves an intent according to opts. The method
937+
// is expected to be run on behalf of a user request, as opposed to a background
938+
// task.
905939
func (ir *IntentResolver) ResolveIntent(
906940
ctx context.Context, intent roachpb.LockUpdate, opts ResolveOptions,
907941
) *kvpb.Error {
908-
return ir.ResolveIntents(ctx, []roachpb.LockUpdate{intent}, opts)
942+
if len(intent.EndKey) == 0 {
943+
// If the caller wants to resolve a single point intent, let it send the
944+
// request immediately. This is a performance optimization to resolve
945+
// conflicting intents immediately for latency-sensitive requests.
946+
//
947+
// We don't set this flag when resolving a range of keys or when resolving
948+
// multiple point intents (in ResolveIntents) due to the limitations around
949+
// pagination described in the comment on ResolveOptions.sendImmediately.
950+
opts.sendImmediately = true
951+
}
952+
return ir.resolveIntents(ctx, (*singleLockUpdate)(&intent), opts)
909953
}
910954

911-
// ResolveIntents synchronously resolves intents according to opts.
955+
// ResolveIntents synchronously resolves intents according to opts. The method
956+
// is expected to be run on behalf of a user request, as opposed to a background
957+
// task.
912958
func (ir *IntentResolver) ResolveIntents(
913959
ctx context.Context, intents []roachpb.LockUpdate, opts ResolveOptions,
914960
) (pErr *kvpb.Error) {
961+
// TODO(nvanbenschoten): unlike IntentResolver.ResolveIntent, we don't set
962+
// sendImmediately on the ResolveOptions here. This is because we don't
963+
// support pagination when sending intent resolution immediately and not
964+
// through the batcher. If this becomes important, we'll need to lift this
965+
// limitation.
915966
return ir.resolveIntents(ctx, (*sliceLockUpdates)(&intents), opts)
916967
}
917968

@@ -943,7 +994,17 @@ func (ir *IntentResolver) resolveIntents(
943994
var singleReq [1]kvpb.Request //gcassert:noescape
944995
reqs := resolveIntentReqs(intents, opts, singleReq[:])
945996

946-
// Send the requests using their corresponding request batcher.
997+
// Send the requests ...
998+
if opts.sendImmediately {
999+
// ... using a single batch.
1000+
b := &kv.Batch{}
1001+
b.AddRawRequest(reqs...)
1002+
if err := ir.db.Run(ctx, b); err != nil {
1003+
return b.MustPErr()
1004+
}
1005+
return nil
1006+
}
1007+
// ... using their corresponding request batcher.
9471008
respChan := make(chan requestbatcher.Response, len(reqs))
9481009
for _, req := range reqs {
9491010
var batcher *requestbatcher.RequestBatcher

0 commit comments

Comments
 (0)