From 06953a51f8ff3aa376b5af85a3f43f5501854c49 Mon Sep 17 00:00:00 2001 From: Marco Peereboom Date: Wed, 18 Dec 2024 13:47:05 +0000 Subject: [PATCH] Provide Running call to see if the service is "up enough" (#340) * Provide Running call to see if the service is "up enough" * Revert " fragile test case + race condition fixes (#337)" This reverts commit c1243e718be9dbcf4ad17f5c4121549a203da504. * only print an error if it fails on close * Revert "Revert " fragile test case + race condition fixes (#337)"" This reverts commit 7e19d722bcff01dbd5ea2f6a2fff383c9b3214e5. * Be a lot more explicit when errors occur on New in database open --- database/level/level.go | 17 +++++++-- go.mod | 2 +- service/tbc/tbc.go | 34 +++++------------- service/tbc/tbc_test.go | 3 -- service/tbc/tbcfork_test.go | 3 -- service/tbc/ulimit_darwin.go | 65 --------------------------------- service/tbc/ulimit_linux.go | 69 ------------------------------------ service/tbc/ulimit_other.go | 13 ------- 8 files changed, 23 insertions(+), 183 deletions(-) delete mode 100644 service/tbc/ulimit_darwin.go delete mode 100644 service/tbc/ulimit_linux.go delete mode 100644 service/tbc/ulimit_other.go diff --git a/database/level/level.go b/database/level/level.go index 7c3c6f88..7ea0258e 100644 --- a/database/level/level.go +++ b/database/level/level.go @@ -163,6 +163,9 @@ func New(ctx context.Context, home string, version int) (*Database, error) { log.Tracef("New") defer log.Tracef("New exit") + // Care must be taken to not shadow err and l in this function. The + // defer will overwrite those if an unwind condition occurs. + h, err := homedir.Expand(home) if err != nil { return nil, fmt.Errorf("home dir: %w", err) @@ -181,7 +184,14 @@ func New(ctx context.Context, home string, version int) (*Database, error) { unwind := true defer func() { if unwind { - log.Errorf("new unwind exited with: %v", l.Close()) + cerr := l.Close() + if cerr != nil { + log.Debugf("new unwind exited with: %v", cerr) + err = errors.Join(err, cerr) + } + clear(l.pool) + clear(l.rawPool) + l = nil // Reset l } }() @@ -240,7 +250,8 @@ func New(ctx context.Context, home string, version int) (*Database, error) { dbVersion, version) } - unwind = false + unwind = false // Everything is good, do not unwind. - return l, nil + // The defer above will set/reset these values. + return l, err } diff --git a/go.mod b/go.mod index 3eef716d..944445c6 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,6 @@ require ( github.com/sethvargo/go-retry v0.3.0 github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 github.com/testcontainers/testcontainers-go v0.32.0 - golang.org/x/sys v0.23.0 ) require ( @@ -90,6 +89,7 @@ require ( go.opentelemetry.io/otel/metric v1.24.0 // indirect go.opentelemetry.io/otel/trace v1.24.0 // indirect golang.org/x/crypto v0.22.0 // indirect + golang.org/x/sys v0.23.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20231016165738-49dd2c1f3d0b // indirect google.golang.org/grpc v1.59.0 // indirect google.golang.org/protobuf v1.34.2 // indirect diff --git a/service/tbc/tbc.go b/service/tbc/tbc.go index 684cd732..4eed5a1a 100644 --- a/service/tbc/tbc.go +++ b/service/tbc/tbc.go @@ -165,10 +165,6 @@ type Server struct { // WebSockets sessions map[string]*tbcWs requestTimeout time.Duration - - // ignoreUlimit will explicitly not check ulimit settings on the host - // machine, this is useful for very small datasets/chains - ignoreUlimit bool } func NewServer(cfg *Config) (*Server, error) { @@ -579,7 +575,7 @@ func (s *Server) handlePeer(ctx context.Context, p *rawpeer.RawPeer) error { } } -func (s *Server) running() bool { +func (s *Server) Running() bool { s.mtx.RLock() defer s.mtx.RUnlock() return s.isRunning @@ -594,7 +590,7 @@ func (s *Server) testAndSetRunning(b bool) bool { } func (s *Server) promRunning() float64 { - r := s.running() + r := s.Running() if r { return 1 } @@ -2228,28 +2224,9 @@ func (s *Server) Run(pctx context.Context) error { return errors.New("run called but External Header mode is enabled") } - if !s.testAndSetRunning(true) { - return errors.New("tbc already running") - } - defer s.testAndSetRunning(false) - - // We need a lot of open files and memory for the indexes. Best effort - // to echo to the user what the ulimits are. - s.ignoreUlimit = true - if s.ignoreUlimit || s.cfg.Network == networkLocalnet { - log.Warningf("ignoring ulimit requirements") - } else if ulimitSupported { - if err := verifyUlimits(); err != nil { - return fmt.Errorf("verify ulimits: %w", err) - } - } else { - log.Errorf("This architecture does not supported ulimit verification. " + - "Consult the README for minimum values.") - } - + // Rely on DBOpen failing if the database is already open. ctx, cancel := context.WithCancel(pctx) defer cancel() - err := s.DBOpen(ctx) if err != nil { return fmt.Errorf("open level database: %w", err) @@ -2261,6 +2238,11 @@ func (s *Server) Run(pctx context.Context) error { } }() + if !s.testAndSetRunning(true) { + return errors.New("tbc already running") + } + defer s.testAndSetRunning(false) + // Find out where IBD is at bhb, err := s.db.BlockHeaderBest(ctx) if err != nil { diff --git a/service/tbc/tbc_test.go b/service/tbc/tbc_test.go index 4b51138a..e9b173a9 100644 --- a/service/tbc/tbc_test.go +++ b/service/tbc/tbc_test.go @@ -834,8 +834,6 @@ func createTbcServer(ctx context.Context, t *testing.T, mappedPeerPort nat.Port) t.Fatal(err) } - tbcServer.ignoreUlimit = true - go func() { err := tbcServer.Run(ctx) if err != nil && !errors.Is(err, context.Canceled) { @@ -1064,7 +1062,6 @@ func createTbcServerExternalHeaderMode(ctx context.Context, t *testing.T) *Serve t.Fatal(err) } - tbcServer.ignoreUlimit = true tbcServer.ExternalHeaderSetup(ctx, defaultUpstreamStateId[:]) return tbcServer } diff --git a/service/tbc/tbcfork_test.go b/service/tbc/tbcfork_test.go index 77ce63f0..51d2d69e 100644 --- a/service/tbc/tbcfork_test.go +++ b/service/tbc/tbcfork_test.go @@ -911,7 +911,6 @@ func TestFork(t *testing.T) { if err != nil { t.Fatal(err) } - s.ignoreUlimit = true go func() { log.Infof("s run") defer log.Infof("s run done") @@ -1149,7 +1148,6 @@ func TestIndexNoFork(t *testing.T) { if err != nil { t.Fatal(err) } - s.ignoreUlimit = true go func() { err := s.Run(ctx) @@ -1322,7 +1320,6 @@ func TestIndexFork(t *testing.T) { if err != nil { t.Fatal(err) } - s.ignoreUlimit = true go func() { err := s.Run(ctx) if err != nil && !errors.Is(err, context.Canceled) && !errors.Is(err, rawpeer.ErrNoConn) { diff --git a/service/tbc/ulimit_darwin.go b/service/tbc/ulimit_darwin.go deleted file mode 100644 index 8889dc0b..00000000 --- a/service/tbc/ulimit_darwin.go +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) 2024 Hemi Labs, Inc. -// Use of this source code is governed by the MIT License, -// which can be found in the LICENSE file. - -//go:build darwin - -package tbc - -import ( - "fmt" - - "golang.org/x/sys/unix" -) - -var ( - resources = []int{ - unix.RLIMIT_AS, - unix.RLIMIT_MEMLOCK, - unix.RLIMIT_NOFILE, - unix.RLIMIT_NPROC, - } - resourceName = map[int]string{ - unix.RLIMIT_AS: "memory", - unix.RLIMIT_MEMLOCK: "lockedmem", - unix.RLIMIT_NOFILE: "nofiles", - unix.RLIMIT_NPROC: "processes", - } - resourceWant = map[int]unix.Rlimit{ - unix.RLIMIT_AS: {Cur: unix.RLIM_INFINITY, Max: unix.RLIM_INFINITY}, - unix.RLIMIT_MEMLOCK: {Cur: 775258112, Max: 775258112}, - unix.RLIMIT_NOFILE: {Cur: 16384, Max: 16384}, - unix.RLIMIT_NPROC: {Cur: 2666, Max: 2666}, - } -) - -const ulimitSupported = true - -func verifyUlimits() error { - var p int - for k, resource := range resources { - var limit unix.Rlimit - if err := unix.Getrlimit(resource, &limit); err != nil { - return fmt.Errorf("ulimit %v: %w", k, err) - } - - // Make sure it is a reasonable value - limitRequired := resourceWant[resource] - if limitRequired.Cur > limit.Cur || limitRequired.Max > limit.Max { - return fmt.Errorf("ulimit %v: limit too low got %v, need %v", - resourceName[resource], limit.Max, limitRequired.Max) - } - - // Echo to user - if err := unix.Getrlimit(resource, &limit); err != nil { - return fmt.Errorf("ulimit %v: %w", k, err) - } - if p == 0 { - log.Infof("%-16v %-22v %-22v", "set resource", "current", "minumum") - p++ - } - log.Infof("%-16v: %-22v %-22v", resourceName[resource], limit.Cur, - limitRequired.Max) - } - return nil -} diff --git a/service/tbc/ulimit_linux.go b/service/tbc/ulimit_linux.go deleted file mode 100644 index e94b74e9..00000000 --- a/service/tbc/ulimit_linux.go +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright (c) 2024 Hemi Labs, Inc. -// Use of this source code is governed by the MIT License, -// which can be found in the LICENSE file. - -//go:build linux - -package tbc - -import ( - "fmt" - "math" - - "golang.org/x/sys/unix" -) - -var ( - resources = []int{ - unix.RLIMIT_AS, - unix.RLIMIT_MEMLOCK, - unix.RLIMIT_NOFILE, - unix.RLIMIT_NPROC, - unix.RLIMIT_RSS, - } - resourceName = map[int]string{ - unix.RLIMIT_AS: "memory", - unix.RLIMIT_MEMLOCK: "lockedmem", - unix.RLIMIT_NOFILE: "nofiles", - unix.RLIMIT_NPROC: "processes", - unix.RLIMIT_RSS: "rss", - } - resourceWant = map[int]unix.Rlimit{ - unix.RLIMIT_AS: {Cur: unix.RLIM_INFINITY, Max: unix.RLIM_INFINITY}, - unix.RLIMIT_MEMLOCK: {Cur: 775254016, Max: 775254016}, - unix.RLIMIT_NOFILE: {Cur: 16384, Max: 16384}, - unix.RLIMIT_NPROC: {Cur: 4196, Max: 4196}, - unix.RLIMIT_RSS: {Cur: math.MaxUint64, Max: math.MaxUint64}, - } -) - -const ulimitSupported = true - -func verifyUlimits() error { - var p int - for k, resource := range resources { - var limit unix.Rlimit - if err := unix.Getrlimit(resource, &limit); err != nil { - return fmt.Errorf("ulimit %v: %w", k, err) - } - - // Make sure it is a reasonable value - limitRequired := resourceWant[resource] - if limitRequired.Cur > limit.Cur || limitRequired.Max > limit.Max { - return fmt.Errorf("ulimit %v: limit too low got %v, need %v", - resourceName[resource], limit.Max, limitRequired.Max) - } - - // Echo to user - if err := unix.Getrlimit(resource, &limit); err != nil { - return fmt.Errorf("ulimit %v: %w", k, err) - } - if p == 0 { - log.Infof("%-16v %-22v %-22v", "set resource", "current", "minumum") - p++ - } - log.Infof("%-16v: %-22v %-22v", resourceName[resource], limit.Cur, - limitRequired.Max) - } - return nil -} diff --git a/service/tbc/ulimit_other.go b/service/tbc/ulimit_other.go deleted file mode 100644 index ecafcd1d..00000000 --- a/service/tbc/ulimit_other.go +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) 2024 Hemi Labs, Inc. -// Use of this source code is governed by the MIT License, -// which can be found in the LICENSE file. - -//go:build !linux && !darwin - -package tbc - -const ulimitSupported = false - -func verifyUlimits() error { - return nil -}