Kraken: Count number of unprefetched messages in queue + increase prefetch #3968
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Increasing prefetched messages should improve retrieval speed. Consume messages until this unprefetched+prefetched count (or max) is reached. This allows for larger timeouts when there are unprefetched messages, as Kraken will stop at that number, and avoid continuing as long as messages arrive (which is constantly the case for some contributors). If there are only prefetched messages, rushing is possible. Also: * Add short "prefetch" timeouts config (in additional to "classic" ones) * Common queue declaration (to avoid bad-redeclaration) * Check (only for log currently) that consumer is unique * minor cleanups of useless variables Notes: * DeclareQueue() uses DeclareQueueWithCounts() under the cover in SimpleAmpqClient, which allows for the common queue declaration refactor. * BasicConsume() declares a consumer that prefetches 1 message by default. DeclareQueueWithCounts doesn't count prefetched messages. Hence tricky max_message_to_retrieve. JIRA: https://navitia.atlassian.net/browse/NAV-1879
pbougue
commented
Apr 6, 2023
| if (queue_is_empty_or_unreachable) { | ||
| LOG4CPLUS_ERROR(logger, | ||
| "Could not retrieve all the messages counted for RabbitMQ's RT queue " << queue_name); | ||
| // going on with what was retrieved so far | ||
| break; |
There was a problem hiding this comment.
An idea by @pbench (correct me if needed) to simplify code and params would be:
- put a
continuehere instead of abreak - only one total_timeout (i.e. 30s), single timeout may be hardcoded
- remove count and just go with what was retrieved during those 30s
This would simplify code, the inconvenient is to have a strict latency even if there is only one message (and tests would need a little parameter adaptation for that reason).
woshilapin
approved these changes
Apr 6, 2023
|
SonarCloud Quality Gate failed. |
pbougue
pushed a commit
that referenced
this pull request
May 3, 2023
As the number of prefetched message is unsure when no ready message is available, avoid logging in this case. improvement after #3968 fix https://github.com/hove-io/navitia-docker-compose/issues/153
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Increasing prefetched messages (default to
100) should improve retrieval speed.Consume messages until this unprefetched+prefetched count (or max - increased to
10000by default) is reached.This allows for larger timeouts when there are unprefetched messages, as Kraken will stop at that number, and avoid continuing as long as messages arrive (which is constantly the case for some contributors). If there are only prefetched messages, rushing is possible.
Also:
100ms and 10s(in additional to "classic" ones - increased to1s and 2min)Notes:
DeclareQueue()usesDeclareQueueWithCounts()under the cover in SimpleAmpqClient, which allows for the common queue declaration refactor.BasicConsume()declares a consumer that prefetches 1 message by default.DeclareQueueWithCountsdoesn't include prefetched messages. Hence trickymax_message_to_retrieve.JIRA: https://navitia.atlassian.net/browse/NAV-1879