Optimize advancing head #337
Conversation
KushnerykPavel
requested review from
tzdybal,
renaynay,
Wondertan,
vgonkivs,
gupadhyaya and
walldiss
as code owners
|
Hello, @Wondertan ! Could you please review ? |
|
Hello, thanks for the ping, I will get to reviewing it around next week. |
There was a problem hiding this comment.
This PR, as it is, doesn't achieve the goal behind the issue. To be frank, it makes it even worse. For every single header, it checks if it exists and reads it up, which is now two reads instead of one. What it should be doing is the following:
- check if nextHeight exists
- and if so, continue until it finds the nextHeight that doesn't
- once we hit a non-existing nextHeight, read the last existing one and return it
- that is, we read the whole header only once and only if hit the wall
| } | ||
|
|
||
| // hasHeaderAt checks if a header exists at the given height by checking both pending headers and the height indexer. | ||
| func (s *Store[H]) hasHeaderAt(ctx context.Context, height uint64) bool { |
There was a problem hiding this comment.
Let's move this implementation into HasAt. The current implementation makes a risky assumption about everything being available between Tail and Head. While logic in go-header aims to have everything between Tail and Head available, it doesn't guarantee that to be the case at all times. Besides, the current HasAt doesn't check for headers outside of the Tail-Head range, which is an allowed case.
| ctx, done := s.withReadTransaction(ctx) | ||
| defer done() | ||
|
|
||
| debugLog := !changed && log.Level() == zapcore.DebugLevel |
|
@KushnerykPavel, do you plan to finish the PR? |
Of course! thank you! |
resolve #332
Add checking header presence.