Update the ResumptionStrategy interface to allow it to modify the incoming response stream. #485
Conversation
…oming response stream. The general use cases for this are: 1. If a stream can only be resumed from a particular point, then its strategy will resume from the closest point, suppressing the repeated messages in the retry 2. If the stream elements have sequence numbers relative to the start of the stream, then strategy can re-write the sequence numbers when it performs a retry. The specific use case is to enable retries for Bigtable's MutateRows RPC. Which has a batch request and streamed responses. Each entry can fail independently of the others. I would like to reuse the streaming retry infrastructure to implement retries. On failure, the strategy will find all of the failed subresponses and compose a new request with them. The responses will have to be rewritten to adjust indexes on the response entries.
Codecov Report
@@ Coverage Diff @@
## master #485 +/- ##
============================================
+ Coverage 70.34% 70.34% +<.01%
- Complexity 773 774 +1
============================================
Files 162 162
Lines 3672 3676 +4
Branches 275 278 +3
============================================
+ Hits 2583 2586 +3
Misses 973 973
- Partials 116 117 +1
Continue to review full report at Codecov.
|
| if (message == null) { | ||
| // Request the next one and exit | ||
| if (!autoFlowControl) { | ||
| innerController.request(1); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@garrettjonesgoogle can you take a look at this? It's in prep for implementing flushing for BulkMutations in the bigtable client |
| * accomplishes two goals: | ||
| * | ||
| * <ol> | ||
| * <li>It allows the strategy implementation to update it's internal state so that it can |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| * <ol> | ||
| * <li>It allows the strategy implementation to update it's internal state so that it can | ||
| * compose the resume request | ||
| * <li>It allows the strategy to alter the incoming responses to adjust for post resume. For |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| innerController.request(1); | ||
| } | ||
| return; | ||
| } |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Would a separate ‘shouldSkip(ResponseT)’ on the strategy be better? I don’t actually care for the suppression functionality, I only need to be able rewrite responses for bigtable. I added it just in case it might be useful. Should I just remove it? |
|
Yeah, if we don't have a need for it yet, let's remove it. |
|
Ok, all feedback should be addressed |
… the incoming response stream. (googleapis#485)" This reverts commit 7d0a0a0.
4 participants
This will allow the streaming retries to be useful in more scenarios.
The general use cases for this are:
The specific use case is to enable retries for Bigtable's MutateRows RPC. Which has a batch request and streamed responses. Each entry can fail independently of the others. I would like to reuse the streaming retry infrastructure to implement retries. On failure, the strategy will find all of the failed subresponses and compose a new request with them. The responses will have to be rewritten to adjust indexes on the response entries.