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.

Sign up for GitHub

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

Jump to bottom

Add failures reason to delete by query response #5095

Closed
wants to merge 1 commit into from
Closed

Add failures reason to delete by query response #5095

wants to merge 1 commit into from

Conversation

kzwang
Copy link
Contributor

@kzwang kzwang commented Feb 12, 2014

closes #5093

It will return an array of failures. e.g.

 "tweet-36": {
    "_shards": {
      "total": 5,
      "successful": 4,
      "failed": 1,
      "failures": [
        {
          "index": "tweet-36",
          "reason": "EsRejectedExecutionException[rejected execution (queue capacity 200) on org.elasticsearch.action.support.replication.TransportShardReplicationOperationAction$AsyncShardOperationAction$1@176685ee]"
        }
      ]
    }
  }

@javanna javanna self-assigned this Feb 12, 2014
javanna
javanna reviewed Feb 13, 2014
View reviewed changes
src/main/java/org/elasticsearch/rest/action/deletebyquery/RestDeleteByQueryAction.java
@@ -109,6 +110,22 @@ public void onResponse(DeleteByQueryResponse result) {
builder.field("total", indexDeleteByQueryResponse.getTotalShards());
builder.field("successful", indexDeleteByQueryResponse.getSuccessfulShards());
builder.field("failed", indexDeleteByQueryResponse.getFailedShards());
ShardOperationFailedException[] failures = indexDeleteByQueryResponse.getFailures();
if (failures != null && failures.length > 0) {
builder.startArray("failures");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you replace the literal strings used here with XContentBuilderString instances, as we do for instance in SearchResponse (introducing an inner Fields class too)?

@javanna
Copy link
Member

javanna commented Feb 13, 2014

I left a small comment, other than that this looks good, thanks!

@kzwang
Copy link
Contributor Author

kzwang commented Feb 13, 2014

Hi @javanna I've changed that to use XContentBuilderString

@javanna
Copy link
Member

javanna commented Feb 14, 2014

Thanks @kzwang ! Just realized that the shard id is never set though (always -1). I'll try to address that before pushing your contribution.

@javanna javanna mentioned this pull request Feb 14, 2014
Merged
@javanna
Copy link
Member

javanna commented Feb 26, 2014

Merged, thanks!

@javanna javanna closed this Feb 26, 2014
@clintongormley clintongormley added >enhancement v1.1.0 v2.0.0-beta1 :Search/Search Search-related issues that do not fall into other categories labels Jun 7, 2015
@clintongormley clintongormley changed the title add failures reason to delete by query response Add failures reason to delete by query response Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@javanna javanna

Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories v1.1.0 v2.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Delete by query doesn't return failure reasons
3 participants
@kzwang @javanna @clintongormley