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

Stop batching polling query fetches. #4800

Merged
merged 1 commit into from
May 10, 2019

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented May 10, 2019

The last time I touched this code in #4243, I thought it was worthwhile to preserve the behavior of sometimes batching polling query fetches in the same tick of the event loop.

However, as @voxtex points out in #4786, almost any batching strategy will lead to unpredictable fetch timing, skipped fetches, poor accounting for fetch duration (which could exceed the polling interval), and so on. On top of that, even the premise that "batching is good" comes into question if it doesn't happen consistently.

An implementation which uses independent timers seems much simpler and more predictable.

The last time I touched this code in #4243, I thought it was worthwhile to
preserve the behavior of sometimes batching polling query fetches in the
same tick of the event loop.

However, as @voxtex points out in #4786, almost any batching strategy will
lead to unpredictable fetch timing, skipped fetches, poor accounting for
fetch duration (which could exceed the polling interval), and so on. On
top of that, even the premise that "batching is good" comes into question
if it doesn't happen consistently.

An implementation which uses independent timers seems much simpler and
more predictable.
@benjamn benjamn added this to the Release 2.6.0 milestone May 10, 2019
@benjamn benjamn requested review from hwillson and jbaxleyiii May 10, 2019 17:22
@benjamn benjamn self-assigned this May 10, 2019
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

I liked the idea of batching, but this is definitely safer. The changes look good @benjamn - thanks!

@benjamn benjamn merged commit 1e425ca into release-2.6.0 May 10, 2019
@benjamn benjamn deleted the stop-batching-polling-query-fetches branch May 10, 2019 18:01
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants