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

Retry mechanism #284

Open
lucavallin opened this issue Apr 18, 2018 · 5 comments

Comments

Projects
None yet
5 participants
@lucavallin
Copy link

commented Apr 18, 2018

Is there a way to retry RPC calls when they fail? I think in the Go package this is available. I couldn't find anything about this in the docs. If not, what approach would you suggest instead?

@nicolasnoble

This comment has been minimized.

Copy link
Collaborator

commented Apr 22, 2018

The retry mechanism hasn't been implemented yet in native core.

@drobertduke

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2018

Retries can be implemented using the interceptor framework, see https://github.com/grpc/grpc-node/blob/master/packages/grpc-native-core/src/client_interceptors.js and https://github.com/grpc/proposal/pull/14/files#diff-c1aee0ddae63a3e9a9ba050796cd4b58R325

The Node interceptor framework will be released with 1.11.0

@TLadd

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2018

@drobertduke @nicolasnoble I tried using the retry interceptor from here, but it didn't work for me as is. After some debugging, it looks like the retry function gets called the first time, and it calls newCall.start here, but the onReceiveMessage and onReceiveStatus listener functions never get triggered; the message isn't sent again.

Looking at the way unary requests are made, it looks like you have to also call sendMessage and halfClose after calling start. I added these calls to the interceptor so it looks like this:

var maxRetries = 3;
var interceptor = function(options, nextCall) {
    var savedMetadata;
    var savedSendMessage;
    var savedReceiveMessage;
    var savedMessageNext;
    var requester = {
        start: function(metadata, listener, next) {
            savedMetadata = metadata;
            var newListener = {
                onReceiveMessage: function(message, next) {
                    savedReceiveMessage = message;
                    savedMessageNext = next;
                },
                onReceiveStatus: function(status, next) {
                    var retries = 0;
                    var retry = function(message, metadata) {
                        retries++;
                        var newCall = nextCall(options);
                        var retryListener = {
                            onReceiveMessage: function(message) {
                                savedReceiveMessage = message;
                            },
                            onReceiveStatus: function(status) {
                                if (status.code !== grpc.status.OK) {
                                    if (retries <= maxRetries) {
                                        retry(message, metadata);
                                    } else {
                                        savedMessageNext(savedReceiveMessage);
                                        next(status);
                                    }
                                } else {
                                    savedMessageNext(savedReceiveMessage);
                                    next({code: grpc.status.OK});
                                }
                            }
                        }
                        newCall.start(metadata, retryListener)
                        newCall.sendMessage(savedSendMessage);  // Added Call
                        newCall.halfClose();  // Added Call
                    };
                    if (status.code !== grpc.status.OK) {
                        retry(savedSendMessage, savedMetadata);
                    } else {
                        savedMessageNext(savedReceiveMessage);
                        next(status);
                    }
                }
            };
            next(metadata, newListener);
        },
        sendMessage: function(message, next) {
            savedSendMessage = message;
            next(message);
        }
    };
    return new InterceptingCall(nextCall(options), requester);
};

And it seems to be working as expected now. Any insight into whether this is correct or not?

@drobertduke

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2018

You are correct, this was an oversight in my original proposal. There is a working version of the retry interceptor in client_interceptors_test.js which includes the calls you've added

@murgatroid99 Should I make a PR to fix up the proposal code?

@murgatroid99

This comment has been minimized.

Copy link
Member

commented Aug 24, 2018

Yes, I think we should keep the proposal correct. And since it is just a correction, it doesn't need the whole review process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.