Skip to content

Commit

Permalink
Remove context lock from API VM interface (#2165)
Browse files Browse the repository at this point in the history
  • Loading branch information
StephenButtolph authored Oct 12, 2023
1 parent 9dbf82a commit 9da2e62
Show file tree
Hide file tree
Showing 22 changed files with 522 additions and 726 deletions.
21 changes: 0 additions & 21 deletions api/server/middleware_handler.go

This file was deleted.

18 changes: 9 additions & 9 deletions api/server/mock_server.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

113 changes: 18 additions & 95 deletions api/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@ package server

import (
"context"
"errors"
"fmt"
"net"
"net/http"
"net/url"
"path"
"sync"
"time"

"github.com/NYTimes/gziphandler"
Expand Down Expand Up @@ -39,15 +37,13 @@ const (
)

var (
errUnknownLockOption = errors.New("invalid lock options")

_ PathAdder = readPathAdder{}
_ Server = (*server)(nil)
)

type PathAdder interface {
// AddRoute registers a route to a handler.
AddRoute(handler *common.HTTPHandler, lock *sync.RWMutex, base, endpoint string) error
AddRoute(handler http.Handler, base, endpoint string) error

// AddAliases registers aliases to the server
AddAliases(endpoint string, aliases ...string) error
Expand All @@ -56,7 +52,7 @@ type PathAdder interface {
type PathAdderWithReadLock interface {
// AddRouteWithReadLock registers a route to a handler assuming the http
// read lock is currently held.
AddRouteWithReadLock(handler *common.HTTPHandler, lock *sync.RWMutex, base, endpoint string) error
AddRouteWithReadLock(handler http.Handler, base, endpoint string) error

// AddAliasesWithReadLock registers aliases to the server assuming the http read
// lock is currently held.
Expand Down Expand Up @@ -182,13 +178,8 @@ func (s *server) Dispatch() error {
}

func (s *server) RegisterChain(chainName string, ctx *snow.ConsensusContext, vm common.VM) {
var (
handlers map[string]*common.HTTPHandler
err error
)

ctx.Lock.Lock()
handlers, err = vm.CreateHandlers(context.TODO())
handlers, err := vm.CreateHandlers(context.TODO())
ctx.Lock.Unlock()
if err != nil {
s.log.Error("failed to create handlers",
Expand Down Expand Up @@ -224,112 +215,44 @@ func (s *server) RegisterChain(chainName string, ctx *snow.ConsensusContext, vm
}
}

func (s *server) addChainRoute(chainName string, handler *common.HTTPHandler, ctx *snow.ConsensusContext, base, endpoint string) error {
func (s *server) addChainRoute(chainName string, handler http.Handler, ctx *snow.ConsensusContext, base, endpoint string) error {
url := fmt.Sprintf("%s/%s", baseURL, base)
s.log.Info("adding route",
zap.String("url", url),
zap.String("endpoint", endpoint),
)
if s.tracingEnabled {
handler = &common.HTTPHandler{
LockOptions: handler.LockOptions,
Handler: api.TraceHandler(handler.Handler, chainName, s.tracer),
}
}
// Apply middleware to grab/release chain's lock before/after calling API method
h, err := lockMiddleware(
handler.Handler,
handler.LockOptions,
s.tracingEnabled,
s.tracer,
&ctx.Lock,
)
if err != nil {
return err
handler = api.TraceHandler(handler, chainName, s.tracer)
}
// Apply middleware to reject calls to the handler before the chain finishes bootstrapping
h = rejectMiddleware(h, ctx)
h = s.metrics.wrapHandler(chainName, h)
return s.router.AddRouter(url, endpoint, h)
handler = rejectMiddleware(handler, ctx)
handler = s.metrics.wrapHandler(chainName, handler)
return s.router.AddRouter(url, endpoint, handler)
}

func (s *server) AddRoute(handler *common.HTTPHandler, lock *sync.RWMutex, base, endpoint string) error {
return s.addRoute(handler, lock, base, endpoint)
func (s *server) AddRoute(handler http.Handler, base, endpoint string) error {
return s.addRoute(handler, base, endpoint)
}

func (s *server) AddRouteWithReadLock(handler *common.HTTPHandler, lock *sync.RWMutex, base, endpoint string) error {
func (s *server) AddRouteWithReadLock(handler http.Handler, base, endpoint string) error {
s.router.lock.RUnlock()
defer s.router.lock.RLock()
return s.addRoute(handler, lock, base, endpoint)
return s.addRoute(handler, base, endpoint)
}

func (s *server) addRoute(handler *common.HTTPHandler, lock *sync.RWMutex, base, endpoint string) error {
func (s *server) addRoute(handler http.Handler, base, endpoint string) error {
url := fmt.Sprintf("%s/%s", baseURL, base)
s.log.Info("adding route",
zap.String("url", url),
zap.String("endpoint", endpoint),
)

if s.tracingEnabled {
handler = &common.HTTPHandler{
LockOptions: handler.LockOptions,
Handler: api.TraceHandler(handler.Handler, url, s.tracer),
}
}

// Apply middleware to grab/release chain's lock before/after calling API method
h, err := lockMiddleware(
handler.Handler,
handler.LockOptions,
s.tracingEnabled,
s.tracer,
lock,
)
if err != nil {
return err
}
h = s.metrics.wrapHandler(base, h)
return s.router.AddRouter(url, endpoint, h)
}

// Wraps a handler by grabbing and releasing a lock before calling the handler.
func lockMiddleware(
handler http.Handler,
lockOption common.LockOption,
tracingEnabled bool,
tracer trace.Tracer,
lock *sync.RWMutex,
) (http.Handler, error) {
var (
name string
lockedHandler http.Handler
)
switch lockOption {
case common.WriteLock:
name = "writeLock"
lockedHandler = middlewareHandler{
before: lock.Lock,
after: lock.Unlock,
handler: handler,
}
case common.ReadLock:
name = "readLock"
lockedHandler = middlewareHandler{
before: lock.RLock,
after: lock.RUnlock,
handler: handler,
}
case common.NoLock:
return handler, nil
default:
return nil, errUnknownLockOption
}

if !tracingEnabled {
return lockedHandler, nil
handler = api.TraceHandler(handler, url, s.tracer)
}

return api.TraceHandler(lockedHandler, name, tracer), nil
handler = s.metrics.wrapHandler(base, handler)
return s.router.AddRouter(url, endpoint, handler)
}

// Reject middleware wraps a handler. If the chain that the context describes is
Expand Down Expand Up @@ -383,8 +306,8 @@ func PathWriterFromWithReadLock(pather PathAdderWithReadLock) PathAdder {
}
}

func (a readPathAdder) AddRoute(handler *common.HTTPHandler, lock *sync.RWMutex, base, endpoint string) error {
return a.pather.AddRouteWithReadLock(handler, lock, base, endpoint)
func (a readPathAdder) AddRoute(handler http.Handler, base, endpoint string) error {
return a.pather.AddRouteWithReadLock(handler, base, endpoint)
}

func (a readPathAdder) AddAliases(endpoint string, aliases ...string) error {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/DataDog/zstd v1.5.2
github.com/Microsoft/go-winio v0.5.2
github.com/NYTimes/gziphandler v1.1.1
github.com/ava-labs/coreth v0.12.5-rc.6
github.com/ava-labs/coreth v0.12.6-rc.0
github.com/ava-labs/ledger-avalanche/go v0.0.0-20230105152938-00a24d05a8c7
github.com/btcsuite/btcd/btcutil v1.1.3
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah
github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM=
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
github.com/ava-labs/coreth v0.12.5-rc.6 h1:OajGUyKkO5Q82XSuMa8T5UD6QywtCHUiZ4Tv3RFmRBU=
github.com/ava-labs/coreth v0.12.5-rc.6/go.mod h1:s5wVyy+5UCCk2m0Tq3jVmy0UqOpKBDYqRE13gInCJVs=
github.com/ava-labs/coreth v0.12.6-rc.0 h1:EaARpccHwa7+P60EXQ/dYUCetZmJjw9RO5L7ebutqXk=
github.com/ava-labs/coreth v0.12.6-rc.0/go.mod h1:sNbwitXv4AhLvWpSqy6V8yzkhGFeWBQFD31/xiRDJ5M=
github.com/ava-labs/ledger-avalanche/go v0.0.0-20230105152938-00a24d05a8c7 h1:EdxD90j5sClfL5Ngpz2TlnbnkNYdFPDXa0jDOjam65c=
github.com/ava-labs/ledger-avalanche/go v0.0.0-20230105152938-00a24d05a8c7/go.mod h1:XhiXSrh90sHUbkERzaxEftCmUz53eCijshDLZ4fByVM=
github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g=
Expand Down
3 changes: 1 addition & 2 deletions indexer/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,7 @@ func (i *indexer) registerChainHelper(
_ = index.Close()
return nil, err
}
handler := &common.HTTPHandler{LockOptions: common.NoLock, Handler: apiServer}
if err := i.pathAdder.AddRoute(handler, &sync.RWMutex{}, "index/"+name, "/"+endpoint); err != nil {
if err := i.pathAdder.AddRoute(apiServer, "index/"+name, "/"+endpoint); err != nil {
_ = index.Close()
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions indexer/indexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package indexer

import (
"errors"
"net/http"
"sync"
"testing"
"time"
Expand All @@ -19,7 +20,6 @@ import (
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/snow"
"github.com/ava-labs/avalanchego/snow/engine/avalanche/vertex"
"github.com/ava-labs/avalanchego/snow/engine/common"
"github.com/ava-labs/avalanchego/snow/engine/snowman/block/mocks"
"github.com/ava-labs/avalanchego/utils"
"github.com/ava-labs/avalanchego/utils/logging"
Expand All @@ -37,7 +37,7 @@ type apiServerMock struct {
endpoints []string
}

func (a *apiServerMock) AddRoute(_ *common.HTTPHandler, _ *sync.RWMutex, base, endpoint string) error {
func (a *apiServerMock) AddRoute(_ http.Handler, base, endpoint string) error {
a.timesCalled++
a.bases = append(a.bases, base)
a.endpoints = append(a.endpoints, endpoint)
Expand Down
Loading

0 comments on commit 9da2e62

Please sign in to comment.