kv: rename UpdateStateOnRemoteRetryableErr to UpdateStateOnRetryableErr

Also add a new RETRY_TABLE_FROM_FUTURE error for a transaction that encounters a lease on a table descriptor such that the write timestamp on the table descriptor is after the transaction timestamp.
vivekmenezes · May 23, 2017 · 9433411 · 9433411
1 parent aba4ad9
commit 9433411
Show file tree

Hide file tree

Showing 5 changed files with 191 additions and 190 deletions.
diff --git a/pkg/internal/client/db_test.go b/pkg/internal/client/db_test.go
@@ -359,54 +359,54 @@ func TestCommonMethods(t *testing.T) {
 		// request with the information that this particular Get must be
 		// unmarshaled, which didn't seem worth doing as we're not using
 		// Batch.GetProto at the moment.
-		{dbType, "GetProto"}:                         {},
-		{txnType, "GetProto"}:                        {},
-		{batchType, "CheckConsistency"}:              {},
-		{batchType, "AddRawRequest"}:                 {},
-		{batchType, "PutInline"}:                     {},
-		{batchType, "RawResponse"}:                   {},
-		{batchType, "MustPErr"}:                      {},
-		{dbType, "AdminMerge"}:                       {},
-		{dbType, "AdminSplit"}:                       {},
-		{dbType, "AdminTransferLease"}:               {},
-		{dbType, "AdminChangeReplicas"}:              {},
-		{dbType, "CheckConsistency"}:                 {},
-		{dbType, "Run"}:                              {},
-		{dbType, "Txn"}:                              {},
-		{dbType, "GetSender"}:                        {},
-		{dbType, "PutInline"}:                        {},
-		{dbType, "WriteBatch"}:                       {},
-		{txnType, "AcceptUnhandledRetryableErrors"}:  {},
-		{txnType, "Commit"}:                          {},
-		{txnType, "CommitInBatch"}:                   {},
-		{txnType, "CommitOrCleanup"}:                 {},
-		{txnType, "Rollback"}:                        {},
-		{txnType, "CleanupOnError"}:                  {},
-		{txnType, "DebugName"}:                       {},
-		{txnType, "EnsureProto"}:                     {},
-		{txnType, "InternalSetPriority"}:             {},
-		{txnType, "IsFinalized"}:                     {},
-		{txnType, "NewBatch"}:                        {},
-		{txnType, "Exec"}:                            {},
-		{txnType, "GetDeadline"}:                     {},
-		{txnType, "ResetDeadline"}:                   {},
-		{txnType, "Run"}:                             {},
-		{txnType, "SetDebugName"}:                    {},
-		{txnType, "SetFixedTimestamp"}:               {},
-		{txnType, "SetIsolation"}:                    {},
-		{txnType, "SetUserPriority"}:                 {},
-		{txnType, "SetSystemConfigTrigger"}:          {},
-		{txnType, "SetTxnAnchorKey"}:                 {},
-		{txnType, "UpdateDeadlineMaybe"}:             {},
-		{txnType, "UpdateStateOnRemoteRetryableErr"}: {},
-		{txnType, "AddCommitTrigger"}:                {},
-		{txnType, "CommandCount"}:                    {},
-		{txnType, "IsRetryableErrMeantForTxn"}:       {},
-		{txnType, "Isolation"}:                       {},
-		{txnType, "OrigTimestamp"}:                   {},
-		{txnType, "Proto"}:                           {},
-		{txnType, "UserPriority"}:                    {},
-		{txnType, "AnchorKey"}:                       {},
+		{dbType, "GetProto"}:                        {},
+		{txnType, "GetProto"}:                       {},
+		{batchType, "CheckConsistency"}:             {},
+		{batchType, "AddRawRequest"}:                {},
+		{batchType, "PutInline"}:                    {},
+		{batchType, "RawResponse"}:                  {},
+		{batchType, "MustPErr"}:                     {},
+		{dbType, "AdminMerge"}:                      {},
+		{dbType, "AdminSplit"}:                      {},
+		{dbType, "AdminTransferLease"}:              {},
+		{dbType, "AdminChangeReplicas"}:             {},
+		{dbType, "CheckConsistency"}:                {},
+		{dbType, "Run"}:                             {},
+		{dbType, "Txn"}:                             {},
+		{dbType, "GetSender"}:                       {},
+		{dbType, "PutInline"}:                       {},
+		{dbType, "WriteBatch"}:                      {},
+		{txnType, "AcceptUnhandledRetryableErrors"}: {},
+		{txnType, "Commit"}:                         {},
+		{txnType, "CommitInBatch"}:                  {},
+		{txnType, "CommitOrCleanup"}:                {},
+		{txnType, "Rollback"}:                       {},
+		{txnType, "CleanupOnError"}:                 {},
+		{txnType, "DebugName"}:                      {},
+		{txnType, "EnsureProto"}:                    {},
+		{txnType, "InternalSetPriority"}:            {},
+		{txnType, "IsFinalized"}:                    {},
+		{txnType, "NewBatch"}:                       {},
+		{txnType, "Exec"}:                           {},
+		{txnType, "GetDeadline"}:                    {},
+		{txnType, "ResetDeadline"}:                  {},
+		{txnType, "Run"}:                            {},
+		{txnType, "SetDebugName"}:                   {},
+		{txnType, "SetFixedTimestamp"}:              {},
+		{txnType, "SetIsolation"}:                   {},
+		{txnType, "SetUserPriority"}:                {},
+		{txnType, "SetSystemConfigTrigger"}:         {},
+		{txnType, "SetTxnAnchorKey"}:                {},
+		{txnType, "UpdateDeadlineMaybe"}:            {},
+		{txnType, "UpdateStateOnRetryableErr"}:      {},
+		{txnType, "AddCommitTrigger"}:               {},
+		{txnType, "CommandCount"}:                   {},
+		{txnType, "IsRetryableErrMeantForTxn"}:      {},
+		{txnType, "Isolation"}:                      {},
+		{txnType, "OrigTimestamp"}:                  {},
+		{txnType, "Proto"}:                          {},
+		{txnType, "UserPriority"}:                   {},
+		{txnType, "AnchorKey"}:                      {},
 	}
 
 	for b := range omittedChecks {

diff --git a/pkg/internal/client/txn.go b/pkg/internal/client/txn.go
@@ -975,12 +975,14 @@ func firstWriteIndex(ba roachpb.BatchRequest) (int, *roachpb.Error) {
 	return firstWriteIdx, nil
 }
 
-// UpdateStateOnRemoteRetryableErr updates the Txn, and the Transaction proto
+// UpdateStateOnRetryableErr updates the Txn, and the Transaction proto
 // inside it, in response to an error encountered when running a request through
 // the txn. If the error is not a RetryableTxnError, then this is a no-op. For a
 // retryable error, the Transaction proto is either initialized with the updated
 // proto from the error, or a new Transaction proto is initialized.
-func (txn *Txn) UpdateStateOnRemoteRetryableErr(ctx context.Context, pErr roachpb.Error) {
+// This needs to be called when a SQL hits a retryable error outside of the
+// TxnCoordSender, like in distSQL or retryable errors in SQL logic.
+func (txn *Txn) UpdateStateOnRetryableErr(ctx context.Context, pErr roachpb.Error) {
 	txn.mu.Lock()
 	defer txn.mu.Unlock()
 
@@ -993,18 +995,6 @@ func (txn *Txn) UpdateStateOnRemoteRetryableErr(ctx context.Context, pErr roachp
 		log.Fatalf(ctx, "unexpected retryable error with no txn ran through DistSQL: %s", pErr)
 	}
 
-	// Assert that the TxnCoordSender doesn't have any state for this transaction
-	// (and it shouldn't, since DistSQL isn't supposed to do any works in
-	// transaction that had performed writes and hence started being tracked). If
-	// the TxnCoordSender were to have state, it'd be a bad thing that we're not
-	// updating it.
-	// TODO(andrei): remove nil check once #15024 is merged.
-	if txnID := pErr.GetTxn().ID; txnID != nil {
-		if _, ok := txn.db.GetSender().(SenderWithDistSQLBackdoor).GetTxnState(*txnID); ok {
-			log.Fatalf(ctx, "unexpected state in TxnCoordSender for transaction in error: %s", pErr)
-		}
-	}
-
 	// Emulate the processing that the TxnCoordSender would have done on this
 	// error.
 	newTxn := roachpb.PrepareTransactionForRetry(ctx, &pErr, txn.mu.UserPriority)