From de735d79113bdcb7ed8fd3e7320c6871405822ba Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Wed, 5 Nov 2025 11:09:32 -0800 Subject: [PATCH] kvclient/kvcoord: remove stale TODO in Step We currently have a TODO in Step to add an assertion that stepping is only done on the RootTxn which references a now-closed issue. I did a little bit archeology, and in bd255e5de633ea99f6fb4e629813b5b3049a106b we consciously removed the assertion like this since stepping on a LeafTxn should be no-op, should just work, and is actually needed behavior for internal executors when they run on remote nodes as part of DistSQL plan. (In connExecutor we currently unconditionally Step the txn on the main query path.) Thus I don't think we want to implement the TODO and should just remove it. Release note: None --- pkg/kv/kvclient/kvcoord/txn_coord_sender.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/kv/kvclient/kvcoord/txn_coord_sender.go b/pkg/kv/kvclient/kvcoord/txn_coord_sender.go index ee9d1891a029..e44450e96f8a 100644 --- a/pkg/kv/kvclient/kvcoord/txn_coord_sender.go +++ b/pkg/kv/kvclient/kvcoord/txn_coord_sender.go @@ -1525,11 +1525,6 @@ func (tc *TxnCoordSender) TestingCloneTxn() *roachpb.Transaction { // Step is part of the TxnSender interface. func (tc *TxnCoordSender) Step(ctx context.Context, allowReadTimestampStep bool) error { - // TODO(nvanbenschoten): it should be possible to make this assertion, but - // the API is currently misused by the connExecutor. See #86162. - // if tc.typ != kv.RootTxn { - // return errors.AssertionFailedf("cannot step in non-root txn") - // } tc.mu.Lock() defer tc.mu.Unlock() if allowReadTimestampStep && tc.shouldStepReadTimestampLocked() {