kv: non-transactional batches can be split into sub-batches #46081
Description
Discussed in #46017 (review)
The following diff on top of #46017 finds a kv.Batch committed via kv.DB that has writes with two different timestamps.
diff --git a/pkg/kv/kvnemesis/generator.go b/pkg/kv/kvnemesis/generator.go
index 6cf485c0b7..b438be4a40 100644
--- a/pkg/kv/kvnemesis/generator.go
+++ b/pkg/kv/kvnemesis/generator.go
@@ -179,7 +179,7 @@ func NewDefaultConfig() GeneratorConfig {
// nontransactional batch are disjoint and upgrading to a transactional batch
// (see CrossRangeTxnWrapperSender) if they are. roachpb.SpanGroup can be used
// to efficiently check this.
- config.Ops.Batch = BatchOperationConfig{}
+ config.Ops.Batch.Ops.PutExisting = 0
return config
} {
b := &Batch{}
b.Put(ctx, /Table/50/"c1556caf", v-5) // nil
b.Get(ctx, /Table/50/"43275686") // (nil, nil)
b.Put(ctx, /Table/50/"0998fa3b", v-6) // nil
db0.Run(ctx, b) // nil
}
committed batch different timestamps: /Table/50/"0998fa3b":1583976139.381769000,0->v-6 /Table/50/"c1556caf":1583976139.381491000,0->v-5
Diagnosis by @nvanbenschoten
we're just straight-up missing a check that non-transactional batches don't get split into sub-batches due to request compatibility rules (as opposed to range boundaries). We'd want something like:
diff --git a/pkg/kv/kvclient/kvcoord/dist_sender.go b/pkg/kv/kvclient/kvcoord/dist_sender.go
index 36954d9867..9ff2620ddd 100644
--- a/pkg/kv/kvclient/kvcoord/dist_sender.go
+++ b/pkg/kv/kvclient/kvcoord/dist_sender.go
@@ -692,6 +693,12 @@ func (ds *DistSender) Send(
splitET = true
}
parts := splitBatchAndCheckForRefreshSpans(ba, splitET)
+ if len(parts) > 1 && ba.Txn == nil && ba.IsTransactional() && ba.ReadConsistency == roachpb.CONSISTENT {
+ // If there's no transaction and ba spans ranges, possibly re-run as part of
+ // a transaction for consistency. The case where we don't need to re-run is
+ // if the read consistency is not required.
+ return nil, roachpb.NewError(&roachpb.OpRequiresTxnError{})
+ }
if len(parts) > 1 && (ba.MaxSpanRequestKeys != 0 || ba.TargetBytes != 0) {
// We already verified above that the batch contains only scan requests of the same type.
// Such a batch should never need splitting.
Found by kvnemesis.
Jira issue: CRDB-5106