Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exposed shard id related to a failure in delete by query #5125

Merged

Conversation

javanna
Copy link
Member

@javanna javanna commented Feb 14, 2014

Relates to #5095, where we want to expose the potential shard failures obtained from the delete by query api. Although the failure is available, the shard id where it happened is not (always -1).

Refactored TransportIndexReplicationOperationAction to be able to expose the shard id related to a shard failure.

The ShardOperationFailedException is now created within TransportIndexReplicationAction passing in the current shard id as a constructor argument.
Also replaced AtomicReferenceArray<Object> with AtomicReferenceArray<ShardActionResult>, where ShardActionResult wraps the ShardResponse or the failure, containing all the needed info.

@javanna javanna self-assigned this Feb 14, 2014
@imotov
Copy link
Contributor

imotov commented Feb 23, 2014

Looks good, but it would be nice to add asserts to DeleteByQueryTests#testFailure to make sure that shard >= 0 in response.failures, since this is the part that you fixed.

@javanna javanna added bug and removed enhancement labels Feb 24, 2014
@javanna
Copy link
Member Author

javanna commented Feb 24, 2014

Makes sense, applied changes suggested by @imotov.

…ose the shard id related to a shard failure

The `ShardOperationFailedException` is now created within `TransportIndexReplicationAction` passing in the current shard id as a constructor argument.
Also replaced `AtomicReferenceArray<Object>` with `AtomicReferenceArray<ShardActionResult>`, where `ShardActionResult` wraps the `ShardResponse` or the failure, containing all the needed info.
@imotov
Copy link
Contributor

imotov commented Feb 26, 2014

LGTM

@javanna javanna merged commit c58c9cd into elastic:master Feb 26, 2014
@javanna javanna added the v1.0.2 label Feb 26, 2014
@clintongormley clintongormley added the :Search/Search Search-related issues that do not fall into other categories label Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v1.0.2 v1.1.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants