You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In Rest.li, Stream doesn’t work well with the retry mechanism.
There are two usages so far.
Backup Request. The Backup Request feature doesn’t work with stream requests. An exception raised because SetReader() is invoked twice for the same EntityStream. Here are the related tickets. The ticket reporter mentioned their requests are fully buffered and only need streaming for the response.
RetryClient. The stream request will be fully recorded in order to replay it. There is no limit on the buffer size, so it can cause potential OOM. Retry is only enabled for specific requests and most of which is already fully buffered in memory.
Potential Fixes
There is a retry logic in RetryClient for a stream request. The implementation is to use a FullEntityObserver to buffer the full request. Every time a request needs to be sent, the RetryClient will initiate a new instance of the stream request with buffered content. We can apply the same logic to the BackupRequestClient.
It feels correct to only support retries for a fully buffered request. Before migrating R2 to full streaming mode, a fully buffered request is implemented as a rest request. We can introduce a new API where we allow the user to send a rest request and to receive a stream response. We will only support retrying for a rest request.
Decision
We agreed Option 2 is better, due to the following reasons.
Semantically it’s correct to only support retries for a rest request and to disallow retries for a streaming request.
Performance-wise, record-and-resend causes additional memory allocations, memory copies, and new instances of Objects including requests, entity streams, etc.
The record-and-resent with stream requests could cause potential OOM if the request body is too large. A limit on buffer size is required. (which is missing for RetryClient.)
Option 2 also simplifies the code by removing the conversion from RestRequest to StreamRequest in the case of rest_over_stream, and special checks around the header IS_FULL_REQUEST. So, it’s good to achieve better maintainability and slightly better performance.
The text was updated successfully, but these errors were encountered:
zizhong
changed the title
Response Only Streaming
Response-only Streaming
Mar 12, 2021
zizhong
pushed a commit
to zizhong/rest.li
that referenced
this issue
Mar 12, 2021
Problem
In Rest.li, Stream doesn’t work well with the retry mechanism.
There are two usages so far.
Potential Fixes
Decision
We agreed Option 2 is better, due to the following reasons.
Performance-wise, record-and-resend causes additional memory allocations, memory copies, and new instances of Objects including requests, entity streams, etc.
The text was updated successfully, but these errors were encountered: