DynamoDB: use of closed network connection (w/ repro) #2737
Comments
chrisr-spectra
added
bug
|
same here
|
|
For what it's worth, I actually get two different stack traces when I run with the tracing code. The one I provided in the description matches the time I saw the closed network connection error, and happens very infrequently. Hover, I frequently get the trace: but it doesn't seem to correspond to an error in my client code. |
|
TYVM for taking the time to repro. I'll try to deep dive this week or early next. |
|
These traces certainly appear to demonstrate that it's net/http that's ultimately closing the conns out from under us. Whether we indirectly caused said close remains to be seen. |
RanVaknin
added
queued
|
@lucix-aws hi, I have a theory but, I do not know how to probe it: I think the real issue is hidden by ignoring the error of closing the request body here https://github.com/aws/smithy-go/blob/bed421c3d7515856512061d107cf2ae3254cf3b8/transport/http/client.go#L116 also I saw a similar issue in the aws-sdk-v1 but the error was "connection closed by peer" maybe the are related with this issue? https://github.com/search?q=repo%3Aaws%2Faws-sdk-go+connection+reset+by+peer&type=issues |
lucix-aws
changed the title
|
Copying this comment I left on the original issue: You can make the underlying error net.ErrClosed retryable in your client with the following: type retryErrClosed struct {
aws.Retryer
}
func (r *retryErrClosed) IsErrorRetryable(err error) bool {
return errors.Is(err, net.ErrClosed) || r.Retryer.IsErrorRetryable(err)
}
svc := s3.NewFromConfig(cfg, func(o *s3.Options) {
o.Retryer = &retryErrClosed{o.Retryer}
})This should be safe to do for get-type operations such as dynamodb.Query. |
|
There's been a lot of back-and-forth about the premature request body close we're doing here. Some have commented claiming that removing it in a fork fixes it for them. At least one person commented that removing it temporarily fixed the issue, but it eventually re-appeared after throughput increased. Unless I've missed something... this is a total red herring. We need to take a closer look at what this code is actually doing in practice. Let's look at Some background: dynamodb.Query is a request-response style operation - we serialize the input structure to the wire and deserialize the HTTP response into the output structure. All of the request/response body Close()ing and such are opaque to the caller. This is different from a streaming-blob style operation like S3 GetObject - where you as the caller literally pass in a stream that we use as the http request body directly. So when you call Query:
So plainly speaking -- the closing behavior that people claim to be problematic is literally a no-op. There is no error or side effect here on the request body close whatsoever, because for request-response style operations, the input is always a byte buffer wrapped in an |
|
@lucix-aws Thanks for the detailed explanation. Based on my recent testing findings, these |
You're experiencing a placebo effect here. Removing that logic does nothing, because the logic itself does nothing as explained above. |
|
In all likelihood this is simply a race in the underlying
The fix is most likely to just add the latter to our set of retryables by default, as my snippet above shows, since these errors are effectively the same thing. Even if they're not -- a retry should get past this anyway. |
|
@lucix-aws Yes retry should solve this, but there's no proper way to categorize the error type. Had to check for certain keywords and contains |
Acknowledgements
go get -u github.com/aws/aws-sdk-go-v2/...)Describe the bug
This is a reproduction of #1825 with the tracing suggested by @lucix-aws
Expected Behavior
dynamodb Client.Query to return without error
Current Behavior
dynamodb Client.Query returns error:
Reproduction Steps
Possible Solution
No response
Additional Information/Context
No response
AWS Go SDK V2 Module Versions Used
Compiler and Version used
go version go1.21.3 linux/amd64
Operating System and version
lambda runtime provided.al2
The text was updated successfully, but these errors were encountered: