Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove context lock from API VM interface #2165

Merged
merged 1 commit into from
Oct 12, 2023
Merged

Remove context lock from API VM interface #2165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions api/server/middleware_handler.go
View file
Open in desktop

This file was deleted.

18 changes: 9 additions & 9 deletions api/server/mock_server.go
View file
Open in desktop

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading