From fb61d7683f1981f421e46ade875d97e8086a794b Mon Sep 17 00:00:00 2001 From: Matthew Sykes Date: Mon, 11 Jun 2018 13:57:31 -0400 Subject: [PATCH] [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 --- core/endorser/endorser.go | 24 ++++++++-- core/endorser/endorser_test.go | 87 ++++++++++++++++++++++++---------- core/mocks/endorser/support.go | 7 ++- 3 files changed, 89 insertions(+), 29 deletions(-) diff --git a/core/endorser/endorser.go b/core/endorser/endorser.go index 72a3866f462..73a5fff07ec 100644 --- a/core/endorser/endorser.go +++ b/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 diff --git a/core/endorser/endorser_test.go b/core/endorser/endorser_test.go index a7941c3120d..d5a81929312 100644 --- a/core/endorser/endorser_test.go +++ b/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,6 +498,7 @@ 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, @@ -505,11 +506,6 @@ func TestEndorserGoodPath(t *testing.T) { 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,6 +521,7 @@ 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, @@ -532,11 +529,6 @@ func TestEndorserLSCC(t *testing.T) { 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,6 +568,7 @@ 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, @@ -583,11 +576,6 @@ func TestEndorseWithPlugin(t *testing.T) { 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) { diff --git a/core/mocks/endorser/support.go b/core/mocks/endorser/support.go index 4907d14b333..42283aceded 100644 --- a/core/mocks/endorser/support.go +++ b/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) {