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

ref(server): Remove overall envelope handling timeout #1398

Merged
merged 4 commits into from
Aug 9, 2022

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Aug 9, 2022

Removes the overall timeout for Envelope buffering and handling,
controlled by the cache.envelope_expiry option. There is a range of
timeouts for project configs and upstream requests that individually
control these processes. The overall buffer is controlled by
cache.envelope_buffer_size.

A reasonable overall time limit is hard to set. In many cases, either
the limit is too short to bridge real downtimes, or it's too long for
the buffer size. In practice, we've been relying on the count-based
limit exclusively. With proper disk-based caching, the limit could be
set to a product-limit such as 30 days.

The option remains in the config and is now marked as unused. It is
likely that some form of time-based expiry is re-introduced once
disk-based caching is implemented.

@jan-auer jan-auer requested a review from a team August 9, 2022 09:26
@jan-auer jan-auer self-assigned this Aug 9, 2022
if let Some(outcome) = outcome {
envelope_context.borrow().send_outcomes(outcome);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

it's kind of sad to be able to use the power of exhaustive matching here. but that's not this PRs fault so just an aside

@jan-auer jan-auer enabled auto-merge (squash) August 9, 2022 11:34
@jan-auer jan-auer merged commit 2732a6a into master Aug 9, 2022
@jan-auer jan-auer deleted the ref/remove-overall-timeout branch August 9, 2022 11:48
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.

None yet

3 participants