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

pubsub: pingStream should not retry to send a message. #1711

Closed
YoshikiShibata opened this issue Dec 26, 2019 · 2 comments
Closed

pubsub: pingStream should not retry to send a message. #1711

YoshikiShibata opened this issue Dec 26, 2019 · 2 comments

Comments

@YoshikiShibata
Copy link

@YoshikiShibata YoshikiShibata commented Dec 26, 2019

Client

pingStream method is defined as following : https://github.com/googleapis/google-cloud-go/blob/master/pubsub/iterator.go#L472

// Send a message to the stream to keep it open. The stream will close if there's no
// traffic on it for a while. By keeping it open, we delay the start of the
// expiration timer on messages that are buffered by gRPC or elsewhere in the
// network. This matters if it takes a long time to process messages relative to the
// default ack deadline, and if the messages are small enough so that many can fit
// into the buffer.
func (it *messageIterator) pingStream() {
	// Ignore error; if the stream is broken, this doesn't matter anyway.
	_ = it.ps.Send(&pb.StreamingPullRequest{})
}

Send() will eventually call call method to send the empty message: https://github.com/googleapis/google-cloud-go/blob/master/pubsub/pullstream.go#L114

Inside the call, if s.get (https://github.com/googleapis/google-cloud-go/blob/master/pubsub/pullstream.go#L129) returns a closed stream, then err = f(*spc) fails, and then there will be a sleep. After the sleep, if s.get returns a closed stream again, this process will be repeated: the call might never return or might return much later: in this situation, pingStream may not return for a while.

pingStream() method should not retry to send the empty message, otherwise, sender() method (https://github.com/googleapis/google-cloud-go/blob/master/pubsub/iterator.go#L260) cannot proceed its loop in some cases.

@hongalex

This comment has been minimized.

Copy link
Member

@hongalex hongalex commented Jan 8, 2020

Thanks for raising this issue! Has pingStream() blocking become an issue for you recently? If the stream is closed (or f(*spc) errors for another reason), then we probably don't want to handle the rest of the items in messageIterator.sender(). We also cap our sleep/backoff at 30s, such that if a stream is unavailable for more than 30s, we will start retrying often. Therefore, we are ready to receive messages as soon as the stream is alive again.

Let me know if there's anything I'm missing from your original description.

@hongalex

This comment has been minimized.

Copy link
Member

@hongalex hongalex commented Jan 13, 2020

Closing for now. Feel free to respond if you disagree with my previous comment and I'll reopen.

@hongalex hongalex closed this Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.