Resolve GCS-fixture batch-delete inconsistencies #139166
Open
+369
−209
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mhl-b
added
>test
Collaborator
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
mhl-b
commented
Dec 6, 2025
Comment on lines
107
to
+115
| new TestHttpResponse(RestStatus.OK, """ | ||
| --$boundary | ||
| Content-Length: 168 | ||
| Content-Type: application/http | ||
| content-id: 1 | ||
| content-transfer-encoding: binary | ||
| content-type: application/http | ||
| content-id: response-1 | ||
| HTTP/1.1 204 NO_CONTENT | ||
| HTTP/1.1 204 No Content | ||
| --$boundary-- | ||
| """.replace("\n", "\r\n").replace("$boundary", boundary)), | ||
| --$boundary--""".replace("\n", "\r\n").replace("$boundary", boundary)), |
Contributor
Author
There was a problem hiding this comment.
Here is a complete example of inconsistencies.
- Content-Length is redundant and wrong, it's 23 bytes + double CRLF.
content-transfer-encoding: binaryis not true.content-id: 1does not have prefixresponse-.- There should be only 2(or even 1) empty lines after
HTTP/1.1 204 No Content
example here https://docs.cloud.google.com/storage/docs/batch#batch-example-response
nicktindall
reviewed
Dec 7, 2025
| var _bucket = matcher.group("bucket"); | ||
| if (bucket.equals(_bucket) == false) { | ||
| throw new IllegalStateException("Bucket does not match expected: " + bucket + ", got: " + _bucket); | ||
| } |
Contributor
There was a problem hiding this comment.
I wonder if we should maybeDieOnAnotherThread if we see what we consider to be a malformed request here. If we just throw an exception here it might retry around the issue somehow and we don't know we've got some issue with our stub.
Contributor
Author
There was a problem hiding this comment.
Thank you, I will add it to the following PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses various inconsistencies in GCS fixture for batch-delete. Source https://docs.cloud.google.com/storage/docs/batch#http.
It is a prerequisite for #138951 to handle retryable errors per batch item. And eventually fix #138364.
This PR addresses following issues:
Content-Length,Content-Type,Content-Transfer-Encoding. These are not required for batch delete responses.Content-IDheader does not align in responses. Responses should have aresponse-prefix with a Content-ID header value from request. For example request:Content-ID: 1, response:Content-ID: response-1.Future work will address remaining issues:
204 No Contenteven if item is not found. In that case it should be404 Not Found.Major change is taking out
Multipart.ReaderfromMultipartUploadand reuse it for batch request. Also addMultipart.Writerfor writing batch responses.