Skip to content
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

fix: Fix bugs in stream message handling and minor logging improvements #438

Merged
merged 4 commits into from
Nov 30, 2023

Conversation

darunrs
Copy link
Collaborator

@darunrs darunrs commented Nov 30, 2023

It was observed that there are three related bugs in stream message processing:

  1. In some cases, the stream will increment the sequence and not the main number, causing incrementId to skip messages.
  2. The current stream ID is used when calling xRange, giving an inaccurate count of messages if any skips occur.
  3. Unprocessed messages are no longer reported when the queue of blocks ie empty. This occasionally led to a non-zero message count being scraped and then left that way even when the stream was in fact zero.

In addition, xRange takes a significant amount of time to run depending on the size of the queue. This impacts all other operations since redis is single-threaded. xLen takes much less time to call while also accurately returning the count of messages in the stream.

To resolve the bugs, I now increment the sequence instead of the main ID. In addition, if no more stream messages are being fetched, the stream message ID is reset to '0' to ensure new messages are collected regardless of their ID. Finally, I replaced xRange with xLen. I also changed a blocking while loop to an if statement in consumer so that if the queue is empty, it continues, which triggers the finally.

I made some small additions to logging statements to include the indexer type and block number for success/failure to help diagnose problems with indexers in the future.

@darunrs darunrs requested a review from a team as a code owner November 30, 2023 19:33
@darunrs darunrs merged commit 98022a1 into main Nov 30, 2023
5 checks passed
@darunrs darunrs deleted the fixIncrementId branch November 30, 2023 20:24
@morgsmccauley morgsmccauley mentioned this pull request Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants