Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: add retry logic to read streams #705

Conversation

stephenplusplus
Copy link
Contributor

Fixes #701

To Dos

file#createReadStream was skipping over the universal response handler logic we have for all API requests our library makes. This is most beneficial for retrying failed requests using the recommended exponential backoff technique. This PR brings retry logic to createReadStream, at the cost of more code complexity, because holy moly, is this tricky with streams.

I wrote up a module that helps with this, retry-request, which also implements exponential backoff the way we had it here.

Here's a simplified example of how this works:
retry-request uses a module called stream-cache that stores what it's given, then replays it when a new stream is connected to it. Additionally, retry-request uses an intermediary stream while we figure out if the request is going to flat out fail. It kind of looks like this (simplified, pseudo):

file.createReadStream = function() {
  var intermediaryStream = new Stream();
  tryRequest();
  return intermediaryStream;

  function tryRequest() {
    var cacheStream = new CacheStream();
    var requestStream = request('http://storage.googleapis.com/bucket/my-file.zip');

    requestStream.pipe(cacheStream);

    requestStream.on('response', function (resp) {
      if (resp.statusCode < 200 || resp.statusCode > 299 && shouldRetry) {
        // Try the request again. All new streams are created, except the
        // intermediary stream (the one the user is holding onto), which still
        // hasn't had any data sent to it.
        tryRequest();
      } else {
        // Okay, no errors here, replay the data we've been caching to the
        // user's stream.
        cacheStream.pipe(intermediaryStream);
      }
    });
  }
}

Related: AWS streams - See last section "Limitations of Streaming"

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 6, 2015
@ryanseys
Copy link
Contributor

ryanseys commented Jul 6, 2015

Streams in node:

rffwukr

Edit:
668

@stephenplusplus
Copy link
Contributor Author

Both excellent choices :)

@stephenplusplus stephenplusplus force-pushed the spp--storage-universal-handler branch 5 times, most recently from bad080e to 5c202b6 Compare July 9, 2015 19:29
@stephenplusplus
Copy link
Contributor Author

don't merge label lifted. This should be safe for a review.

@stephenplusplus stephenplusplus force-pushed the spp--storage-universal-handler branch 2 times, most recently from 64f7209 to 21ff797 Compare July 15, 2015 14:57
@stephenplusplus
Copy link
Contributor Author

@callmehiphop can you take a look over this and let me know if anything looks 👍 or 👎?


return;
callback = callback || util.noop;

This comment was marked as spam.

This comment was marked as spam.

@stephenplusplus stephenplusplus force-pushed the spp--storage-universal-handler branch from 21ff797 to d439aac Compare July 15, 2015 15:46
@callmehiphop
Copy link
Contributor

LGTM! 👍 @stephenplusplus

@stephenplusplus stephenplusplus force-pushed the spp--storage-universal-handler branch from d439aac to 68a4878 Compare July 15, 2015 15:49
stephenplusplus added a commit that referenced this pull request Jul 15, 2015
…andler

core: add retry logic to read streams
@stephenplusplus stephenplusplus merged commit f72a307 into googleapis:master Jul 15, 2015
@stephenplusplus
Copy link
Contributor Author

Thanks!

sofisl pushed a commit that referenced this pull request Jan 24, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
sofisl pushed a commit that referenced this pull request Jan 25, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
sofisl pushed a commit that referenced this pull request Sep 13, 2023
samples: pull in latest typeless bot, clean up some comments

Source-Link: https://github.com/googleapis/synthtool/commit/0a68e568b6911b60bb6fd452eba4848b176031d8
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:5b05f26103855c3a15433141389c478d1d3fe088fb5d4e3217c4793f6b3f245e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants