From 234278daa045f1153ab292ed6952806bdf5237b5 Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Fri, 19 Jul 2024 14:13:52 +0200 Subject: [PATCH 1/4] Configurable timeout for HeaderReader polling Previously if the backend stopped responding the client.HeaderByNumber call inside the timed main loop would never return, meaning that if the HeaderReader was only operating in polling mode, calls to LastHeader would always return the last successfully fetched header until OldHeaderTimeout, and if that was set to a high duration then it would effectively always return the last successfully fetched header and never an error. This would lead to the chain appear to be stuck and not advancing to clients of HeaderReader.LastHeader. This change adds a configurable timeout on this call, defaulting to 5s. Other places in HeaderReader that use LastHeader use the passed-in context which the client can control, so no extra timeout was added for these. --- util/headerreader/header_reader.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/util/headerreader/header_reader.go b/util/headerreader/header_reader.go index 06dfcfbfa8..074d24338e 100644 --- a/util/headerreader/header_reader.go +++ b/util/headerreader/header_reader.go @@ -63,6 +63,7 @@ type Config struct { Enable bool `koanf:"enable"` PollOnly bool `koanf:"poll-only" reload:"hot"` PollInterval time.Duration `koanf:"poll-interval" reload:"hot"` + PollTimeout time.Duration `koanf:"poll-timeout" reload:"hot"` SubscribeErrInterval time.Duration `koanf:"subscribe-err-interval" reload:"hot"` TxTimeout time.Duration `koanf:"tx-timeout" reload:"hot"` OldHeaderTimeout time.Duration `koanf:"old-header-timeout" reload:"hot"` @@ -80,6 +81,7 @@ var DefaultConfig = Config{ Enable: true, PollOnly: false, PollInterval: 15 * time.Second, + PollTimeout: 5 * time.Second, SubscribeErrInterval: 5 * time.Minute, TxTimeout: 5 * time.Minute, OldHeaderTimeout: 5 * time.Minute, @@ -94,6 +96,7 @@ func AddOptions(prefix string, f *flag.FlagSet) { f.Bool(prefix+".poll-only", DefaultConfig.PollOnly, "do not attempt to subscribe to header events") f.Bool(prefix+".use-finality-data", DefaultConfig.UseFinalityData, "use l1 data about finalized/safe blocks") f.Duration(prefix+".poll-interval", DefaultConfig.PollInterval, "interval when polling endpoint") + f.Duration(prefix+".poll-timeout", DefaultConfig.PollTimeout, "timeout when polling endpoint") f.Duration(prefix+".subscribe-err-interval", DefaultConfig.SubscribeErrInterval, "interval for subscribe error") f.Duration(prefix+".tx-timeout", DefaultConfig.TxTimeout, "timeout when waiting for a transaction") f.Duration(prefix+".old-header-timeout", DefaultConfig.OldHeaderTimeout, "warns if the latest l1 block is at least this old") @@ -108,6 +111,7 @@ var TestConfig = Config{ Enable: true, PollOnly: false, PollInterval: time.Millisecond * 10, + PollTimeout: time.Second * 5, TxTimeout: time.Second * 5, OldHeaderTimeout: 5 * time.Minute, UseFinalityData: false, @@ -287,7 +291,9 @@ func (s *HeaderReader) broadcastLoop(ctx context.Context) { s.possiblyBroadcast(h) timer.Stop() case <-timer.C: - h, err := s.client.HeaderByNumber(ctx, nil) + timedCtx, cancelFunc := context.WithTimeout(ctx, s.config().PollTimeout) + h, err := s.client.HeaderByNumber(timedCtx, nil) + cancelFunc() if err != nil { s.setError(fmt.Errorf("failed reading HeaderByNumber: %w", err)) if !errors.Is(err, context.Canceled) { From 4fdae71a9d139a55d84e902958235272f777dab9 Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Mon, 22 Jul 2024 13:41:58 +0200 Subject: [PATCH 2/4] Fallback to trying old layout --- das/local_file_storage_service.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/das/local_file_storage_service.go b/das/local_file_storage_service.go index 621cf3efdb..cad323b165 100644 --- a/das/local_file_storage_service.go +++ b/das/local_file_storage_service.go @@ -110,17 +110,21 @@ func (s *LocalFileStorageService) Close(ctx context.Context) error { func (s *LocalFileStorageService) GetByHash(ctx context.Context, key common.Hash) ([]byte, error) { log.Trace("das.LocalFileStorageService.GetByHash", "key", pretty.PrettyHash(key), "this", s) - var batchPath string - if s.enableLegacyLayout { - batchPath = s.legacyLayout.batchPath(key) - } else { - batchPath = s.layout.batchPath(key) - } + + legacyBatchPath := s.legacyLayout.batchPath(key) + batchPath := s.layout.batchPath(key) data, err := os.ReadFile(batchPath) if err != nil { if errors.Is(err, os.ErrNotExist) { - return nil, ErrNotFound + data, err = os.ReadFile(legacyBatchPath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, ErrNotFound + } + return nil, err + } + return data, nil } return nil, err } From 903c937d9eb62c7009f74555f89c999469405285 Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Mon, 22 Jul 2024 14:23:20 +0200 Subject: [PATCH 3/4] Fix health check spamming the expiry index --- das/local_file_storage_service.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/das/local_file_storage_service.go b/das/local_file_storage_service.go index cad323b165..5623195657 100644 --- a/das/local_file_storage_service.go +++ b/das/local_file_storage_service.go @@ -229,7 +229,14 @@ func (s *LocalFileStorageService) String() string { func (s *LocalFileStorageService) HealthCheck(ctx context.Context) error { testData := []byte("Test-Data") - err := s.Put(ctx, testData, uint64(time.Now().Add(time.Minute).Unix())) + // Store some data with an expiry time at the start of the epoch. + // If expiry is disabled it will only create an index entry for the + // same timestamp each time the health check happens. + // If expiry is enabled, it will be cleaned up each time the pruning + // runs. There is a slight chance of a race between pruning and the + // Put and Get calls, but systems using the HealthCheck will just retry + // and succeed the next time. + err := s.Put(ctx, testData /* start of epoch */, 0) if err != nil { return err } From df93f6045d3c5b7a80f7de71ba503128eee9ec17 Mon Sep 17 00:00:00 2001 From: Tsahi Zidenberg Date: Mon, 22 Jul 2024 18:12:22 -0600 Subject: [PATCH 4/4] update geth pin to 1.13.15 --- go-ethereum | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go-ethereum b/go-ethereum index 18256c2dfc..48de2030c7 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit 18256c2dfcce8fd567aa05e03fbc11a4c17aa550 +Subproject commit 48de2030c7a6fa8689bc0a0212ebca2a0c73e3ad