feat(relayer): add leader streamer #2351
Conversation
corverroos
changed the title
| } | ||
|
|
||
| // IncRange increases the range to the provided height plus one | ||
| // since the leader will move on the that height next. |
There was a problem hiding this comment.
| // since the leader will move on the that height next. | |
| // since the leader will move on that height next. |
| for s := range b.leaders { | ||
| if s.contains(offset) { | ||
| return nil, false | ||
| } | ||
| } |
There was a problem hiding this comment.
I think you planned to use containsStreamerUnsafe here.
| if attestOffset == 1 { | ||
| // Block other streams | ||
| <-ctx.Done() | ||
| return ctx.Err() | ||
| } |
There was a problem hiding this comment.
Not sure I understand the case attestOffset == 1. Maybe worth a comment?
| type leader struct { | ||
| mu sync.RWMutex | ||
| from uint64 | ||
| latest uint64 |
There was a problem hiding this comment.
It feels like latest is not really needed because it's only used to guess if the range was covered by somebody else or not (this does not directly prove that the range is consistenty filled). So in case of some invariant violation (for whatever reason) we can get a leader with inconsistent range coverage that will prevent others from starting on its range.
Instead we could not rely on ranges at all and simply start a new leader on any height that is a cache miss — it will ultimately bump into somebody's range (or timeout).
There was a problem hiding this comment.
nope, this is actually critical, since, latest is increased as soon as previous is fetched, this ensure that this leader remains the leader and others don't also start streaming from next as soon as it isn't in the cache.
|
I do realise an issue with design though: “blast radius”:
Two options:
|
Implements a leader based attest stream cache that aims to avoid overlapping streams thereby deduping the query load on the archive node.
issue: #2321