Skip to content

Conversation

@davidtsuk
Copy link
Contributor

Instead of touching the health file every time we call poll, we only touch it when one of the following two conditions is true:

  1. We have a commit request which means we are actively processing messages
  2. We are calling poll after not calling submit in the previous iteration - this happens when we don't have messages to process which is a healthy state

@davidtsuk davidtsuk marked this pull request as ready for review May 1, 2025 22:38
@davidtsuk davidtsuk requested review from a team as code owners May 1, 2025 22:38
}

fn submit(&mut self, message: Message<TPayload>) -> Result<(), SubmitError<TPayload>> {
self.iterations_since_last_submit = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure if we should check if submit was successful before reseting this counter

path: PathBuf,
interval: Duration,
deadline: SystemTime,
iterations_since_last_submit: u64,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using a u64, but maybe we can get away with a u32

Copy link
Member

@evanh evanh left a comment

Choose a reason for hiding this comment

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

Can you clarify in your commit message when we would end up in the situation where those two conditions are not met?

Also, can you add tests?

@fpacifici
Copy link
Contributor

As this is something you are testing for the Snuba consumer I would do this inside the snuba consumer code rather than the standard HealthCheck in the arroyo code base.
That would allow you to have runtime options to turn this behavior on and off.
This approach would be a breaking change for other consumers as well.

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.

4 participants