Skip to content

Commit

Permalink
chore(sync)!: Rename unrecentHead to outdatedHead in metrics + cl…
Browse files Browse the repository at this point in the history
…ean up `Head` func a bit (#163)

This PR renames a poorly named metric from `unrecent_header` to `outdated_head` which is more descriptive / accurate. 

It also cleans up a bit of the code inside of `sync_head.go`.

This PR is meant to override #162 if we agree that the timeout should not be configurable (see [comment](#155 (comment))
  • Loading branch information
renaynay committed Mar 14, 2024
1 parent 06f57bb commit b9fa9aa
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 16 deletions.
14 changes: 7 additions & 7 deletions sync/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type metrics struct {

syncLoopStarted metric.Int64Counter
trustedPeersOutOfSync metric.Int64Counter
unrecentHeader metric.Int64Counter
outdatedHeader metric.Int64Counter
subjectiveInit metric.Int64Counter

subjectiveHead atomic.Int64
Expand Down Expand Up @@ -53,9 +53,9 @@ func newMetrics() (*metrics, error) {
return nil, err
}

unrecentHeader, err := meter.Int64Counter(
"hdr_sync_unrecent_header_counter",
metric.WithDescription("tracks every time Syncer returns an unrecent header"),
outdatedHeader, err := meter.Int64Counter(
"hdr_sync_outdated_head_counter",
metric.WithDescription("tracks every time Syncer returns an outdated head"),
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -110,7 +110,7 @@ func newMetrics() (*metrics, error) {
m := &metrics{
syncLoopStarted: syncLoopStarted,
trustedPeersOutOfSync: trustedPeersOutOfSync,
unrecentHeader: unrecentHeader,
outdatedHeader: outdatedHeader,
subjectiveInit: subjectiveInit,
syncLoopDurationHist: syncLoopDurationHist,
syncLoopRunningInst: syncLoopRunningInst,
Expand Down Expand Up @@ -148,9 +148,9 @@ func (m *metrics) syncFinished(ctx context.Context) {
})
}

func (m *metrics) unrecentHead(ctx context.Context) {
func (m *metrics) outdatedHead(ctx context.Context) {
m.observe(ctx, func(ctx context.Context) {
m.unrecentHeader.Add(ctx, 1)
m.outdatedHeader.Add(ctx, 1)
})
}

Expand Down
20 changes: 11 additions & 9 deletions sync/sync_head.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import (
"github.com/celestiaorg/go-header"
)

// headRequestTimeout is the amount of time the syncer is willing to wait for
// the exchange to request the head of the chain from the network.
var headRequestTimeout = time.Second * 2

// Head returns the Network Head.
//
// Known subjective head is considered network head if it is recent enough(now-timestamp<=blocktime)
Expand All @@ -24,27 +28,25 @@ func (s *Syncer[H]) Head(ctx context.Context, _ ...header.HeadOption[H]) (H, err
if isRecent(sbjHead, s.Params.blockTime, s.Params.recencyThreshold) {
return sbjHead, nil
}
// otherwise, request head from the network
// TODO: Besides requesting we should listen for new gossiped headers and cancel request if so
//
// single-flight protection
// ensure only one Head is requested at the time

// single-flight protection ensure only one Head is requested at the time
if !s.getter.Lock() {
// means that other routine held the lock and set the subjective head for us,
// so just recursively get it
return s.Head(ctx)
}
defer s.getter.Unlock()
// limit time to get a recent header
// if we can't get it - give what we have
reqCtx, cancel := context.WithTimeout(ctx, time.Second*2) // TODO(@vgonkivs): make timeout configurable

s.metrics.outdatedHead(s.ctx)

reqCtx, cancel := context.WithTimeout(ctx, headRequestTimeout)
defer cancel()
s.metrics.unrecentHead(s.ctx)
netHead, err := s.getter.Head(reqCtx, header.WithTrustedHead[H](sbjHead))
if err != nil {
log.Warnw("failed to get recent head, returning current subjective", "sbjHead", sbjHead.Height(), "err", err)
return s.subjectiveHead(ctx)
}

// process and validate netHead fetched from trusted peers
// NOTE: We could trust the netHead like we do during 'automatic subjective initialization'
// but in this case our subjective head is not expired, so we should verify netHead
Expand Down

0 comments on commit b9fa9aa

Please sign in to comment.