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

Stream retries #729

Open
schmidt-sebastian opened this issue Feb 25, 2020 · 7 comments
Open

Stream retries #729

schmidt-sebastian opened this issue Feb 25, 2020 · 7 comments
Assignees
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@schmidt-sebastian
Copy link
Contributor

The NodeJS Firestore SDK currently has some logic that tries to detect whether a stream is healthy. If an error occurs before a stream has received a response message, the Firestore SDK retries its idempotent streams transparently.

I would like to explore the option of adding this retry behavior directly to GAX, which would allow us to simplify the Firestore SDK, increase the reliability for all other NodeJS SDKs and expose the same backoff behavior for all request types.

For reference, the Firestore implementation lives here: https://github.com/googleapis/nodejs-firestore/blob/master/dev/src/index.ts#L1170

A general retry mechanism for unary streams has recently been added to Python's GAX: googleapis/google-cloud-python#10206

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Feb 26, 2020
@bcoe bcoe added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Feb 26, 2020
@bcoe
Copy link
Contributor

bcoe commented Feb 26, 2020

is this a duplicate of: #726

@yoshi-automation yoshi-automation removed the triage me I really want to be triaged. label Feb 26, 2020
@schmidt-sebastian
Copy link
Contributor Author

#726 might be a prerequisite, but what I would like is for GAX to follow the retry behavior that is specified in our client config (https://github.com/googleapis/nodejs-firestore/blob/master/dev/src/v1/firestore_client_config.json#L70) automatically. Ideally, GAX would apply the same retry here as it does for request/response methods.

@danieljbruce
Copy link
Contributor

Maybe you mean this line of code https://github.com/googleapis/nodejs-firestore/blob/release-v4.12.4/dev/src/v1/firestore_client_config.json#L26 since this describes the retry behaviour?

@danieljbruce
Copy link
Contributor

Since this is gax based, looping @alexander-fenster in. I guess this means firestore would also benefit from retry logic work in google-gax.

@leahecole
Copy link
Contributor

Server streaming retry logic was added as an opt-in feature in #1496 and I would love to see nodejs-firestore opting into it. @danieljbruce are you the one to work with on this once we're done with CBT?

@danieljbruce
Copy link
Contributor

@leahecole I have done work on nodejs-datastore, but not nodejs-firestore.

@leahecole
Copy link
Contributor

Hey @MarkDuckworth - I think that you work on nodejs-firestore these days and I see that @schmidt-sebastian is OOO and also working on TF stuff. I would be super interested in trying to help the node firestore SDK adopt this server streaming retries implementation - are you the right person to work with on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

6 participants