Use dynamic publish requests limits based on subscriptions and latency#408
Open
svanharmelen wants to merge 1 commit intolocka99:masterfrom
Open
Use dynamic publish requests limits based on subscriptions and latency#408svanharmelen wants to merge 1 commit intolocka99:masterfrom
svanharmelen wants to merge 1 commit intolocka99:masterfrom
Conversation
einarmo
approved these changes
Dec 4, 2024
Contributor
einarmo
left a comment
There was a problem hiding this comment.
This solution seems good to me, as we discussed. The "big number" in the sender queue is maybe a little questionable, but the reasoning is sound. I'll think about a better solution in the future, maybe.
This file contains hidden or 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
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.
This PR tries to incorporate both publishing strategies as described in the spec:
It follows the first strategy by sending a publish request right after a subscription is created and immediately responding to a publish response with a new request if the number of inflight publish requests is less then the currently active subscriptions times two (so that there is always one available at the server while the client is processing the other one). So this (active subscriptions times two) defines the
min_publish_requests.In most low-latency environments this is enough to make sure the server always has a publish request available. Even if the server had a temporary performance issue (e.g. due to a backup or other external process) and wasn't able to send the publish response within the specified window, it is able to "catch up" due to the fact that the client will immediately send a new publish request instead of waiting for a certain interval to have passed.
But in case the network between your client and the sever has a high latency, this approach alone is not enough to make sure there are enough publish requests available to the server to be able to send continuous publish responses. So in addition to the above we also keep track of the current network latency by measuring how long the round-trip of the keep-alive messages take. We divide the measured round-trip time by the lowest publish interval of the active subscriptions and multiple that by the
min_publish_requeststo define themax_publish_requests.Changes in the
max_publish_requestswill be handled by checking at every publish interval. When the timer ticks it will check if the total number of in flight requests in below themax_publish_requestsand if so, send an additional publish request.We checked this approach against different types of servers and with different types of networks (with fluctuating latency) and the combined strategy was able to adjust itself in all tested situations so that there always was a steady flow of publish requests and responses.