Skip to content

Conversation

@jmank88
Copy link
Contributor

@jmank88 jmank88 commented Feb 18, 2022

@jmank88 jmank88 requested a review from Nic0S February 18, 2022 18:12
@github-actions
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

// manually by someone who knows what he is doing
b.backfillBlockNumber.SetValid(blockNumber)
b.logger.Debugw("Returning from the event loop to replay logs from specific block number", "blockNumber", blockNumber)
b.onReplayRequest(blockNumber)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have it above, do we still need it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because we only check above, but then sit and wait here. We still need to yield to replay also.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah good point

}

select {
case rawLog := <-chRawLogs:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if we get something on the replayChannel, but there is already a message waiting on the chRawLogs channel (from before the replay). Would that cause issues once the replay starts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC the replay should only be delayed by one log in the worst case (or one other action here in general, e.g. head), then it would loop again and necessarily select the replay in the priority select.

@jmank88 jmank88 force-pushed the sc-28587-prioritize-replay branch from 5a33dc0 to 2719049 Compare February 18, 2022 19:59
@jmank88 jmank88 added the ready for review PR is ready for code review label Feb 22, 2022
@jmank88 jmank88 requested a review from makramkd February 23, 2022 15:46
@jmank88 jmank88 marked this pull request as ready for review February 28, 2022 17:34
@jmank88 jmank88 merged commit ffd2a0c into develop Feb 28, 2022
@jmank88 jmank88 deleted the sc-28587-prioritize-replay branch February 28, 2022 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review PR is ready for code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants