PutObject doesn't work (if able to retry)

Question

PutObject doesn't work (if able to retry)

ahrens opened this issue a year ago · 2 comments

Background

The objectstorage.PutObjectRequest.PutObjectBody which is provided to (objectstorage.ObjectStorageClient).PutObject(...) needs to implement io.Seeker in order for the request to be retried.

Problem

PutObject() does not work, when the PutObjectBody implements Seeker. It hangs until the server closes the connection or the retry limit is reached, without ever issuing a request to the server.

Example

        req := objectstorage.PutObjectRequest{
		NamespaceName: &namespace,
		BucketName:    &bucket,
		ObjectName:    &objectName,
		PutObjectBody: io.NopCloser(bytes.NewReader(data)), // Reader implements Seeker
	}
	client.PutObject(context.Background(), req) // does not issue request, hangs

The following is seen in the log, note http: ContentLength=189 with Body length 0:

DEBUG 2023/05/17 16:17:33.470481 objectstorage_client.go:2450: Attempting to call downstream service
DEBUG 2023/05/17 16:17:33.474010 client.go:692: net/http: HTTP/1.x transport connection broken: http: ContentLength=189 with Body length 0
INFO 2023/05/17 16:17:33.596602 log.go:229: Put "https://objectstorage.us-phoenix-1.oraclecloud.com/n/axrerambs334/b/dcoa-prod-mahrens-oci/o/zfs/agents/b04ad169-52f7-4dd6-8208-bf595f05abc1": net/http: HTTP/1.x transport connection broken: http: ContentLength=189 with Body length 0
DEBUG 2023/05/17 16:17:33.596637 asm_amd64.s:1598: waiting 1.755s before retrying operation
DEBUG 2023/05/17 16:17:35.352555 asm_amd64.s:1598: operation attempt #2

Diagnosis

The problem seems to be that when the SDK calculates the size of the body, it reads the entire PutObjectBody, and then when it tries to send the body, it finds that the PutObjectBody has zero length (because it's already been read to the end). Perhaps it should instead seek the PutObjectBody back the the beginning. See getNormalBinaryBodyLength().

I diagnosed the problem by using a custom struct that implements Reader and Seeker, add adds additional printf's when its functions are called. I see that there is special case code in checkBinaryBodyLength() for handling NopCloser(Reader) types, but the problem occurs for both custom structs as well as NopCloser(Reader).

Workaround

The problem goes away if the objectstorage.PutObjectRequest.ContentLength is provided. See structToRequestPart(). e.g..

        req := objectstorage.PutObjectRequest{
		NamespaceName: &namespace,
		BucketName:    &bucket,
		ObjectName:    &objectName,
		ContentLength: &len64, // marked `mandatory:"false"` but actually required to workaround the bug
		PutObjectBody: io.NopCloser(bytes.NewReader(data)), // Reader implements Seeker
	}
	client.PutObject(context.Background(), req) // works

Additional concerns

When the PutObjectBody is Seeker, could you calculate its size by seeking to the end, determining the position, and then seeking back to where it started? That way you don't have to read the entire body into a buffer, and the PutObjectBody is left in its original state, so subsequent code continues to work as expected.
It would be nice if the PutObject docs mentioned that the PutObjectBody needs to implement Seeker, assuming you want it to be retried.
Although it would be a breaking change, it might be a good idea for the PutObjectBody to be required to implement Seeker. That way consumers can't accidentally get unexpected behavior (no retries on this operation despite configuring retries).