[FAB-10540] stop getting tx sim for qscc/cscc

The state database transaction manager uses a read/write lock to
serialize access to the state database. FAB-7595 introduced an
additional lock in the block storage layer to prevent stale reads from
the block access APIs after a commit event.

During block commit processing, an exclusive lock is acquired at the
block storage layer. While this lock is held, the state database
transaction manager is called to perform its commit processing.

During commit processing, the state database transaction manager
attempts to acquire exclusive access to its commit lock and blocks while
waiting for the associated readers to complete. At this layer, read
locks are held by open transaction simulators associated with chaincode
invocations.

In the case of qscc, since it's chaincode, a transaction simulator is
obtained for it and a read lock is held while is running. If the query
happens to execute concurrent to commit processing, qscc will be unable
to access the block storage layer because of the exclusive lock held
during commit. The commit processing fails to complete as it is unable
to acquire exclusive access to the commit lock maintained by the state
database transaction manager because the query chaincode is currently
holding the lock as a reader.

This commit removes the acquisition of a transaction simulator for the
query and configuration system chaincode as they do not require the
simulator. This prevents the read lock from getting obtained by the
state database transaction manager.

Change-Id: Icde801b583b2bf8a2c9953a6c160b0478816ef64
Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

Author: sykesm

core/endorser/endorser.go

@@ -408,8 +408,7 @@ func (e *Endorser) preProcess(signedProp *pb.SignedProposal) (*validateResult, e
 
 	// block invocations to security-sensitive system chaincodes
 	if e.s.IsSysCCAndNotInvokableExternal(hdrExt.ChaincodeId.Name) {
-		endorserLogger.Errorf("Error: an attempt was made by %#v to invoke system chaincode %s",
-			shdr.Creator, hdrExt.ChaincodeId.Name)
+		endorserLogger.Errorf("Error: an attempt was made by %#v to invoke system chaincode %s", shdr.Creator, hdrExt.ChaincodeId.Name)
 		err = errors.Errorf("chaincode %s cannot be invoked through a proposal", hdrExt.ChaincodeId.Name)
 		vr.resp = &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}
 		return vr, err
@@ -468,10 +467,11 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
 	// Also obtain a history query executor for history queries, since tx simulator does not cover history
 	var txsim ledger.TxSimulator
 	var historyQueryExecutor ledger.HistoryQueryExecutor
-	if chainID != "" {
+	if acquireTxSimulator(chainID, vr.hdrExt.ChaincodeId) {
 		if txsim, err = e.s.GetTxSimulator(chainID, txid); err != nil {
 			return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, nil
 		}
+
 		// txsim acquires a shared lock on the stateDB. As this would impact the block commits (i.e., commit
 		// of valid write-sets to the stateDB), we must release the lock as early as possible.
 		// Hence, this txsim object is closed in simulateProposal() as soon as the tx is simulated and
@@ -547,6 +547,24 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
 	return pResp, nil
 }
 
+// determine whether or not a transaction simulator should be
+// obtained for a proposal.
+func acquireTxSimulator(chainID string, ccid *pb.ChaincodeID) bool {
+	if chainID == "" {
+		return false
+	}
+
+	// ¯\_(ツ)_/¯ locking.
+	// Don't get a simulator for the query and config system chaincode.
+	// These don't need the simulator and its read lock results in deadlocks.
+	switch ccid.Name {
+	case "qscc", "cscc":
+		return false
+	default:
+		return true
+	}
+}
+
 // shorttxid replicates the chaincode package function to shorten txids.
 // ~~TODO utilize a common shorttxid utility across packages.~~
 // TODO use a formal type for transaction ID and make it a stringer

core/endorser/endorser_test.go

@@ -66,6 +66,14 @@ func getSignedPropWithCHIdAndArgs(chid, ccid, ccver string, ccargs [][]byte, t *
 	return &pb.SignedProposal{ProposalBytes: propBytes, Signature: signature}
 }
 
+func newMockTxSim() *mockccprovider.MockTxSim {
+	return &mockccprovider.MockTxSim{
+		GetTxSimulationResultsRv: &ledger.TxSimulationResults{
+			PubSimulationResults: &rwset.TxReadWriteSet{},
+		},
+	}
+}
+
 func TestEndorserNilProp(t *testing.T) {
 	es := endorser.NewEndorserServer(pvtEmptyDistributor, &em.MockSupport{
 		GetApplicationConfigBoolRv: true,
@@ -172,6 +180,7 @@ func TestEndorserSysCC(t *testing.T) {
 	m := &mock.Mock{}
 	m.On("Sign", mock.Anything).Return([]byte{1, 2, 3, 4, 5}, nil)
 	m.On("Serialize").Return([]byte{1, 1, 1}, nil)
+	m.On("GetTxSimulator", mock.Anything, mock.Anything).Return(newMockTxSim(), nil)
 	support := &em.MockSupport{
 		Mock: m,
 		GetApplicationConfigBoolRv: true,
@@ -180,11 +189,6 @@ func TestEndorserSysCC(t *testing.T) {
 		IsSysCCRv:                  true,
 		ChaincodeDefinitionRv:      &ccprovider.ChaincodeData{Escc: "ESCC"},
 		ExecuteResp:                &pb.Response{Status: 200, Payload: utils.MarshalOrPanic(&pb.ProposalResponse{Response: &pb.Response{}})},
-		GetTxSimulatorRv: &mockccprovider.MockTxSim{
-			GetTxSimulationResultsRv: &ledger.TxSimulationResults{
-				PubSimulationResults: &rwset.TxReadWriteSet{},
-			},
-		},
 	}
 	attachPluginEndorser(support)
 	es := endorser.NewEndorserServer(pvtEmptyDistributor, support)
@@ -448,6 +452,7 @@ func TestEndorserGoodPathWEvents(t *testing.T) {
 	m := &mock.Mock{}
 	m.On("Sign", mock.Anything).Return([]byte{1, 2, 3, 4, 5}, nil)
 	m.On("Serialize").Return([]byte{1, 1, 1}, nil)
+	m.On("GetTxSimulator", mock.Anything, mock.Anything).Return(newMockTxSim(), nil)
 	support := &em.MockSupport{
 		Mock: m,
 		GetApplicationConfigBoolRv: true,
@@ -456,11 +461,6 @@ func TestEndorserGoodPathWEvents(t *testing.T) {
 		ChaincodeDefinitionRv:      &ccprovider.ChaincodeData{Escc: "ESCC"},
 		ExecuteResp:                &pb.Response{Status: 200, Payload: utils.MarshalOrPanic(&pb.ProposalResponse{Response: &pb.Response{}})},
 		ExecuteEvent:               &pb.ChaincodeEvent{},
-		GetTxSimulatorRv: &mockccprovider.MockTxSim{
-			GetTxSimulationResultsRv: &ledger.TxSimulationResults{
-				PubSimulationResults: &rwset.TxReadWriteSet{},
-			},
-		},
 	}
 	attachPluginEndorser(support)
 	es := endorser.NewEndorserServer(pvtEmptyDistributor, support)
@@ -498,18 +498,14 @@ func TestEndorserGoodPath(t *testing.T) {
 	m := &mock.Mock{}
 	m.On("Sign", mock.Anything).Return([]byte{1, 2, 3, 4, 5}, nil)
 	m.On("Serialize").Return([]byte{1, 1, 1}, nil)
+	m.On("GetTxSimulator", mock.Anything, mock.Anything).Return(newMockTxSim(), nil)
 	support := &em.MockSupport{
 		Mock: m,
 		GetApplicationConfigBoolRv: true,
 		GetApplicationConfigRv:     &mc.MockApplication{CapabilitiesRv: &mc.MockApplicationCapabilities{}},
 		GetTransactionByIDErr:      errors.New(""),
 		ChaincodeDefinitionRv:      &ccprovider.ChaincodeData{Escc: "ESCC"},
 		ExecuteResp:                &pb.Response{Status: 200, Payload: utils.MarshalOrPanic(&pb.ProposalResponse{Response: &pb.Response{}})},
-		GetTxSimulatorRv: &mockccprovider.MockTxSim{
-			GetTxSimulationResultsRv: &ledger.TxSimulationResults{
-				PubSimulationResults: &rwset.TxReadWriteSet{},
-			},
-		},
 	}
 	attachPluginEndorser(support)
 	es := endorser.NewEndorserServer(pvtEmptyDistributor, support)
@@ -525,18 +521,14 @@ func TestEndorserLSCC(t *testing.T) {
 	m := &mock.Mock{}
 	m.On("Sign", mock.Anything).Return([]byte{1, 2, 3, 4, 5}, nil)
 	m.On("Serialize").Return([]byte{1, 1, 1}, nil)
+	m.On("GetTxSimulator", mock.Anything, mock.Anything).Return(newMockTxSim(), nil)
 	support := &em.MockSupport{
 		Mock: m,
 		GetApplicationConfigBoolRv: true,
 		GetApplicationConfigRv:     &mc.MockApplication{CapabilitiesRv: &mc.MockApplicationCapabilities{}},
 		GetTransactionByIDErr:      errors.New(""),
 		ChaincodeDefinitionRv:      &ccprovider.ChaincodeData{Escc: "ESCC"},
 		ExecuteResp:                &pb.Response{Status: 200, Payload: utils.MarshalOrPanic(&pb.ProposalResponse{Response: &pb.Response{}})},
-		GetTxSimulatorRv: &mockccprovider.MockTxSim{
-			GetTxSimulationResultsRv: &ledger.TxSimulationResults{
-				PubSimulationResults: &rwset.TxReadWriteSet{},
-			},
-		},
 	}
 	attachPluginEndorser(support)
 	es := endorser.NewEndorserServer(pvtEmptyDistributor, support)
@@ -576,18 +568,14 @@ func TestEndorseWithPlugin(t *testing.T) {
 	m := &mock.Mock{}
 	m.On("Sign", mock.Anything).Return([]byte{1, 2, 3, 4, 5}, nil)
 	m.On("Serialize").Return([]byte{1, 1, 1}, nil)
+	m.On("GetTxSimulator", mock.Anything, mock.Anything).Return(newMockTxSim(), nil)
 	support := &em.MockSupport{
 		Mock: m,
 		GetApplicationConfigBoolRv: true,
 		GetApplicationConfigRv:     &mc.MockApplication{CapabilitiesRv: &mc.MockApplicationCapabilities{}},
 		GetTransactionByIDErr:      errors.New("can't find this transaction in the index"),
 		ChaincodeDefinitionRv:      &resourceconfig.MockChaincodeDefinition{EndorsementStr: "ESCC"},
 		ExecuteResp:                &pb.Response{Status: 200, Payload: []byte{1}},
-		GetTxSimulatorRv: &mockccprovider.MockTxSim{
-			GetTxSimulationResultsRv: &ledger.TxSimulationResults{
-				PubSimulationResults: &rwset.TxReadWriteSet{},
-			},
-		},
 	}
 	attachPluginEndorser(support)
 
@@ -648,6 +636,55 @@ func TestEndorserJavaChecks(t *testing.T) {
 	assert.Error(t, err)
 }
 
+func TestEndorserAcquireTxSimulator(t *testing.T) {
+	tc := []struct {
+		name          string
+		chainID       string
+		chaincodeName string
+		simAcquired   bool
+	}{
+		{"empty channel", "", "ignored", false},
+		{"query scc", util.GetTestChainID(), "qscc", false},
+		{"config scc", util.GetTestChainID(), "cscc", false},
+		{"mainline", util.GetTestChainID(), "chaincode", true},
+	}
+
+	expectedResponse := &pb.Response{Status: 200, Payload: utils.MarshalOrPanic(&pb.ProposalResponse{Response: &pb.Response{}})}
+	for _, tt := range tc {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			m := &mock.Mock{}
+			m.On("Sign", mock.Anything).Return([]byte{1, 2, 3, 4, 5}, nil)
+			m.On("Serialize").Return([]byte{1, 1, 1}, nil)
+			m.On("GetTxSimulator", mock.Anything, mock.Anything).Return(newMockTxSim(), nil)
+			support := &em.MockSupport{
+				Mock: m,
+				GetApplicationConfigBoolRv: true,
+				GetApplicationConfigRv:     &mc.MockApplication{CapabilitiesRv: &mc.MockApplicationCapabilities{}},
+				GetTransactionByIDErr:      errors.New(""),
+				ChaincodeDefinitionRv:      &ccprovider.ChaincodeData{Escc: "ESCC"},
+				ExecuteResp:                expectedResponse,
+			}
+			attachPluginEndorser(support)
+			es := endorser.NewEndorserServer(pvtEmptyDistributor, support)
+
+			t.Parallel()
+			args := [][]byte{[]byte("args")}
+			signedProp := getSignedPropWithCHIdAndArgs(tt.chainID, tt.chaincodeName, "version", args, t)
+
+			resp, err := es.ProcessProposal(context.Background(), signedProp)
+			assert.NoError(t, err)
+			assert.Equal(t, expectedResponse, resp.Response)
+
+			if tt.simAcquired {
+				m.AssertCalled(t, "GetTxSimulator", mock.Anything, mock.Anything)
+			} else {
+				m.AssertNotCalled(t, "GetTxSimulator", mock.Anything, mock.Anything)
+			}
+		})
+	}
+}
+
 var signer msp.SigningIdentity
 
 func TestMain(m *testing.M) {

core/mocks/endorser/support.go

@@ -66,7 +66,12 @@ func (s *MockSupport) IsSysCCAndNotInvokableExternal(name string) bool {
 }
 
 func (s *MockSupport) GetTxSimulator(ledgername string, txid string) (ledger.TxSimulator, error) {
-	return s.GetTxSimulatorRv, s.GetTxSimulatorErr
+	if s.Mock == nil {
+		return s.GetTxSimulatorRv, s.GetTxSimulatorErr
+	}
+
+	args := s.Called(ledgername, txid)
+	return args.Get(0).(ledger.TxSimulator), args.Error(1)
 }
 
 func (s *MockSupport) GetHistoryQueryExecutor(ledgername string) (ledger.HistoryQueryExecutor, error) {