Skip to content

Commit

Permalink
[FAB-10540] stop getting tx sim for qscc/cscc
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@sykesm
sykesm committed Jun 14, 2018
1 parent 4236764 commit fb61d76
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 29 deletions.
24 changes: 21 additions & 3 deletions core/endorser/endorser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
87 changes: 62 additions & 25 deletions core/endorser/endorser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 6 additions & 1 deletion core/mocks/endorser/support.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit fb61d76

Please sign in to comment.