Skip to content

Conversation

@wirew0rm
Copy link
Member

@wirew0rm wirew0rm commented Jul 1, 2025

PR #371 introduced that long polling subscriptions would always fetch the next n (defaulted to 3) long polling indices to reduce the latency from roundtrip to single trip.
For the happy path this works well, but in case of errors, the logic has to be improved to correctly handle responses arriving out of order. With the current logic this will re-fetch already received updates and lead to avalanches of requests, as these re-fetched requests will then also again arive out of order and refetch old data. Before increasing this value again, it should be ensured that the responses are either re-sorted on arrival or older updates correctly dropped without scheduling new requests.

This change restores the behaviour from before the changes but keeps the logic in place so it can be reenabled after fixing the error-handling.

PR #371 introduced that long polling subscriptions would always fetch the
next n (defaulted to 3) long polling indices to reduce the latency from
roundtrip to single trip.
For the happy path this works well, but in case of errors, the logic has
to be improved to correctly handle responses arriving out of order.
With the current logic this will re-fetch already received updates and
lead to avalanches of requests, as these re-fetched requests will then
also again arive out of order and refetch old data.
Before increasing this value again, it should be ensured that the
responses are either re-sorted on arrival or older updates correctly
dropped without scheduling new requests.

This change restores the behaviour from before the changes but keeps the
logic in place so it can be reenabled after fixing the error-handling.

Signed-off-by: Alexander Krimm <[email protected]>
@wirew0rm wirew0rm temporarily deployed to configure coverage July 1, 2025 14:57 — with GitHub Actions Inactive
@wirew0rm wirew0rm temporarily deployed to configure coverage July 1, 2025 14:57 — with GitHub Actions Inactive
@wirew0rm wirew0rm temporarily deployed to configure coverage July 1, 2025 14:57 — with GitHub Actions Inactive
@wirew0rm wirew0rm had a problem deploying to configure coverage July 1, 2025 14:57 — with GitHub Actions Failure
@wirew0rm wirew0rm temporarily deployed to configure coverage July 1, 2025 14:57 — with GitHub Actions Inactive
@wirew0rm wirew0rm temporarily deployed to configure coverage July 1, 2025 14:57 — with GitHub Actions Inactive
Copy link
Contributor

@drslebedev drslebedev left a comment

Choose a reason for hiding this comment

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

Looks good and can be merged as discussed.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 1, 2025

@wirew0rm wirew0rm merged commit 579738f into main Jul 1, 2025
9 of 10 checks passed
@wirew0rm wirew0rm deleted the httpClientSingleRequest branch July 1, 2025 15:32
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.

3 participants