Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
logs are forwarded to a processor in slot and trx order #953
logs are forwarded to a processor in slot and trx order #953
Changes from all commits
c7ef582
ac40ee8
d901b20
28e8dea
c54eb2a
f5bf83f
4bc6b10
7e2241d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: if quantity is 0 - signal that bock is ready
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see... you'll probably want to delay the
heap.Pop
call I have above till here, so that if you break or return early it's not already removed. Instead of the initialheap.Pop
you can "peak" at the minimum block number on the heap withblock := (*p.blocks)[0]
. Even though the rest may be out of order, this should always be guaranteed to be the min element.I think the two places you'll need the
heap.Pop
are in place of the 2 instances ofrmvIdx = append(rmvIndex, idx)
, does that sound right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure the
heap.Pop
call is useful in this instance though. Pop will remove the first element, but all I want to do is iterate through them in order without alterations. Then, when I want to remove specific indexes, Pop will not work. I need to useheap.Remove
to specify which index to remove.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'm missing something, but is there a reason why you can't just Pop each block as soon as you've verified that the expectations are met? I don't see any case where you go back and look at old blocks or look ahead at future blocks, so I'm not sure why we have to flag the block for removal and then go back through later and locate all the ones flagged for removal? If we're done with it, why not just discard it and move on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to Domino's comment. I do not see a reason to defer removal from blocks once we've verified that block met expectations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should pass to the processor the whole block and all related events instead of a single event.
This way, it knows that the whole block was processed instead of a single event. (On the startup we won't need to refetch data for the block of latest log).
Also, for finality detection work, it would be helpful to store even empty blocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Domino brought this up as well in conversation and she will be doing the interface change as a followup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't even thought of the reasons @dhaidashenko gives, just was worried that writing 1 event at a time would put more strain on the db than writing batches at a time as they become available.