Skip to content

fix(bigtable): retry unexpected EOF errors#11276

Closed
martin-sucha wants to merge 2 commits into
googleapis:mainfrom
kiwicom:ms/retry-unexpected-eof-upstream
Closed

fix(bigtable): retry unexpected EOF errors#11276
martin-sucha wants to merge 2 commits into
googleapis:mainfrom
kiwicom:ms/retry-unexpected-eof-upstream

Conversation

@martin-sucha

Copy link
Copy Markdown
Contributor

We see occasional unexpected EOF errors returned from ReadRows. These errors are coming from the stream.RecvMsg call.

We have seen similar error messages also from some GCS calls. I assume the underlying TCP connection is terminated while gRPC is reading a message from the wire.

I don't know the exact place where in the gRPC library the error originates, but considering that a RST_STREAM is retried, it seems that we should retry also unexpected EOF.

We see occasional unexpected EOF errors returned from ReadRows.
These errors are coming from the stream.RecvMsg call.

We have seen similar error messages also from some GCS calls.
I assume the underlying TCP connection is terminated
while gRPC is reading a message from the wire.

I don't know the exact place where in the gRPC library the error
originates, but considering that a RST_STREAM is retried,
it seems that we should retry also unexpected EOF.
@martin-sucha martin-sucha requested review from a team December 12, 2024 10:00
@product-auto-label product-auto-label Bot added the api: bigtable Issues related to the Bigtable API. label Dec 12, 2024
@martin-sucha

Copy link
Copy Markdown
Contributor Author

Please let me know if you think ErrUnexpectedError should be checked by value instead of the error message.

@mutianf

mutianf commented Jan 7, 2025

Copy link
Copy Markdown
Contributor

We know when RST_STREAM will occur so it's ok to check on this error. However, I'm not sure when ErrUnexpectedEOF will be thrown, so I'm not sure if we should blindly retry it. Is there any way for us to reproduce this erro?

@mutianf

mutianf commented Jan 8, 2025

Copy link
Copy Markdown
Contributor

Looks like compute and bigquery also retries ErrUnexpectedEOF error. I'm ok with this fix, but I think we should check on the ErrUnexpectedError instead of the message.

@martin-sucha

Copy link
Copy Markdown
Contributor Author

I don't know how to reproduce.

However, I have found another error in logs that might be related: received 4294967294-bytes data exceeding the limit 131070 bytes. It seems that grpc-go closes the stream with io.EOF in that case:

https://github.com/grpc/grpc-go/blob/d0bf90aeb9b5bdf4031d812dbb743b0eb616c7b2/internal/transport/http2_client.go#L1203-L1203

https://github.com/grpc/grpc-go/blob/d0bf90aeb9b5bdf4031d812dbb743b0eb616c7b2/internal/transport/http2_client.go#L1203-L1203

So it could theoretically be connected. Will try to get more information, but it could take a few days until the error appears again.

@mutianf

mutianf commented Jan 9, 2025

Copy link
Copy Markdown
Contributor

If ErrUnexpectedEOF could be coming from received 4294967294-bytes data exceeding the limit 131070 bytes, it shouldn't be retried, because it means that some read failed and it's not a network transient error.

@mutianf

mutianf commented Jan 10, 2025

Copy link
Copy Markdown
Contributor

Closing this PR, because I think in Bigtable's case ErrUnexpectedEOF could be thrown with actual read error and we shouldn't hide the error message.

@mutianf mutianf closed this Jan 10, 2025
@martin-sucha

Copy link
Copy Markdown
Contributor Author

Just an update here, after we upgraded grpc-go, we no longer see the received 4294967294-bytes data exceeding the limit 131070 bytes, but we still see the unexpected EOF, so I think the unexpected EOF is caused by some network component.

bhshkh pushed a commit that referenced this pull request May 30, 2026
This has been previously discussed at
#11276.
I am resubmitting the change again, since we still see the UnexpectedEOF
and we are running with this patch in production to retry it.

Apparently there is a flaky test that shows the same issue as well:
#10909

There are more cases that use the comparison of the error message now,
and the grpc-go has [some
places](https://github.com/grpc/grpc-go/blob/ae62635cb0e21ae37e7ae5f0f18aaac5e8139834/rpc_util.go#L1033)
that return a gRPC status error with the error message of the
io.UnexpectedEOF.
So I kept the handling by error message text instead of comparing
directly to io.UnexpectedEOF.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants