Skip to content

Commit

Permalink
[OTE-180] Parallelize query support.
Browse files Browse the repository at this point in the history
  • Loading branch information
lcwik committed Feb 22, 2024
1 parent 24873b5 commit 75a6d5f
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 11 deletions.
27 changes: 16 additions & 11 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ func (app *BaseApp) Info(_ *abci.RequestInfo) (*abci.ResponseInfo, error) {
// Query implements the ABCI interface. It delegates to CommitMultiStore if it
// implements Queryable.
func (app *BaseApp) Query(_ context.Context, req *abci.RequestQuery) (resp *abci.ResponseQuery, err error) {
app.mtx.Lock()
defer app.mtx.Unlock()
app.mtx.RLock()
defer app.mtx.RUnlock()

// add panic recovery for all queries
//
Expand Down Expand Up @@ -1027,11 +1027,8 @@ func handleQueryApp(app *BaseApp, path []string, req *abci.RequestQuery) *abci.R
case "simulate":
txBytes := req.Data

// Simulate enters runTx which requires us to give up the lock now.
app.mtx.Unlock()
// Simulate must not recursively re-enter and attempt to acquire `app.mtx`.
gInfo, res, err := app.Simulate(txBytes)
app.mtx.Lock()

if err != nil {
return sdkerrors.QueryResult(errorsmod.Wrap(err, "failed to simulate tx"), app.trace)
}
Expand Down Expand Up @@ -1179,11 +1176,8 @@ func (app *BaseApp) handleQueryGRPC(handler GRPCQueryHandler, req *abci.RequestQ
return sdkerrors.QueryResult(err, app.trace)
}

// handler recursively can re-enter Query which requires us to give up the lock now.
app.mtx.Unlock()
// handler must not recursively re-enter and attempt to acquire `app.mtx`.
resp, err := handler(ctx, req)
app.mtx.Lock()

if err != nil {
resp = sdkerrors.QueryResult(gRPCErrorToSDKError(err), app.trace)
resp.Height = req.Height
Expand Down Expand Up @@ -1264,7 +1258,18 @@ func (app *BaseApp) CreateQueryContext(height int64, prove bool) (sdk.Context, e
)
}

cacheMS, err := qms.CacheMultiStoreWithVersion(height)
// Acquire an exclusive lock to ensure that the multistore can be versioned.
// Internally the multistore updates version ranges while loading which leads to data races.
// If there are upstream changes to the multistore to make it use locks effectively
// in this scenario then we could remove this lock.
var cacheMS storetypes.CacheMultiStore
var err error
func() {
app.cacheMsWithVersionMtx.Lock()
defer app.cacheMsWithVersionMtx.Unlock()
cacheMS, err = qms.CacheMultiStoreWithVersion(height)
}()

if err != nil {
return sdk.Context{},
errorsmod.Wrapf(
Expand Down
3 changes: 3 additions & 0 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ type BaseApp struct {

// Used to synchronize the application when using an unsynchronized ABCI++ client.
mtx sync.RWMutex
// Used to synchronize CacheMultistoreWithVersion since the multistore mutates version
// information internally during first time loads leading to data races.
cacheMsWithVersionMtx sync.Mutex
}

// NewBaseApp returns a reference to an initialized BaseApp. It accepts a
Expand Down

0 comments on commit 75a6d5f

Please sign in to comment.