Add client retry support to .map #2571
Closed
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
rohansingh
force-pushed
the
rohan/retry-map
branch
6 times, most recently
from
00957f4 to
55349c4
Compare
rohansingh
force-pushed
the
rohan/retry-map
branch
8 times, most recently
from
85f41bb to
bd1ee57
Compare
rohansingh
force-pushed
the
rohan/retry-map
branch
from
bd1ee57 to
e7ba872
Compare
When two items had the same timestamp, we would try to sort by the actual item value, which breaks for types that don't support comparison. Instead use a nonce when inserting an item, to ensure that we never have to compare the item value itself.
rohansingh
force-pushed
the
rohan/retry-map
branch
from
e7ba872 to
cf8b561
Compare
Though very unlikely outside of unit tests, it's possible to have an output returned before the corresponding retry context has been put into the `pending_outputs` dict.
Once the input queue filled up, we had no more room to put pending retries. And since we had no more room to put retries, we stopped fetching new outputs. And since we stopped fetching new outputs, the server stopped accepting new inputs. As a result, the input queue would never burn down. Instead, use a semaphore to ensure we never have more than 1000 items outstanding.
Instead of using a priority queue, just use the event loop to schedule retries in the future. This significantly simplifies the implementation and makes it much more like the original. Note that we still do have a semaphore that ensures that no more than 1K inputs are in flight (i.e., sent to the server but not completed).
rohansingh
force-pushed
the
rohan/retry-map
branch
from
6ed9306 to
eb82dd4
Compare
|
Continued in #2734. |
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.
Implements similar logic for
.mapas #2403 did for.remote.To do this, we create a
TimedPriorityQueue, which holds all pending inputs and pending retries, along with the timestamp at which they should be sent to the server.When inputs are first read from the generator, they are added to the queue with a timestamp of "now". And if a failed output is received from the server, it's added back to the queue with a configured retry delay.