Failed preparsing does not fail whole bulk request #4781
Merged
Conversation
This file contains 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
| @@ -264,7 +264,7 @@ public void readFrom(StreamInput in) throws IOException { | |||
| if (in.readBoolean()) { | |||
| String fIndex = in.readSharedString(); | |||
| String fType = in.readSharedString(); | |||
| String fId = in.readString(); | |||
| String fId = in.readOptionalString(); | |||
There was a problem hiding this comment.
well I guess that was a problem before so we don't need conditional reads / writes here depending on the version? I wonder when this can be null?
There was a problem hiding this comment.
This can be null, if you try to extract the ID from a broken JSON and thus fail.
This hasnt popped up before, because the whole request failed and so there was no need to send this single BulkItemResponse over the wire.
|
LGTM - added a small comment |
If a preparsing of the source is needed (due to mapping configuration, which extracts the routing/id value from the source) and the source is not valid JSON, then the whole bulk request is failed instead of a single BulkRequest. This commit ensures, that a broken JSON request is not forwarded to the destination shard and creates an appropriate BulkItemResponse, which includes a failure. This also implied changing the BulkItemResponse serialization, because one cannot be sure anymore, if a response includes an ID, in case it was not specified and could not be extracted from the JSON. Closes elastic#4745
clintongormley
changed the title
lcawl
added
:Distributed/CRUD
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
If a preparsing of the source is needed (due to mapping configuration,
which extracts the routing/id value from the source) and the source is not
valid JSON, then the whole bulk request is failed instead of a single
BulkRequest.
This commit ensures, that a broken JSON request is not forwarded to the
destination shard and creates an appropriate BulkItemResponse, which
includes a failure.
This also implied changing the BulkItemResponse serialization, because one
cannot be sure anymore, if a response includes an ID, in case it was not
specified and could not be extracted from the JSON.
Closes #4745