Improve Close Index Response #39687

tlrx · 2019-03-05T10:01:08Z

This changes the CloseIndexResponse so that it reports closing result for each index. Shard failures or exception are also reported per index, and the global acknowledgment flag is computed from the index results only.

The response looks like:

{
  "acknowledged" : true,
  "shards_acknowledged" : true,
  "indices" : {
    "docs" : {
      "closed" : true
    }
  }
}

The response reports shard failures like:

{
  "acknowledged" : false,
  "shards_acknowledged" : false,
  "indices" : {
    "docs-1" : {
      "closed" : true
    },
    "docs-2" : {
      "closed" : false,
      "shards" : {
        "1" : {
          "failures" : [
            {
              "shard" : 1,
              "index" : "docs-2",
              "status" : "BAD_REQUEST",
              "reason" : {
                "type" : "index_closed_exception",
                "reason" : "closed",
                "index_uuid" : "JFmQwr_aSPiZbkAH_KEF7A",
                "index" : "docs-2"
              }
            }
          ]
        }
      }
    },
    "docs-3" : {
      "closed" : true
    }
  }
}

Relates #33888

elasticmachine · 2019-03-05T10:01:11Z

Pinging @elastic/es-distributed

ywelsch

Looking good. Can @henningandersen give this a look as well? Thank you

henningandersen

Thanks @dnhatn , I left a few comments, my primary concern is the comment on handling when closeRoutingTable skips indices.

server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponse.java

henningandersen · 2019-04-30T10:16:48Z

server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java

-                                                .map(result -> result.getKey().getName())
-                                                .filter(index -> newState.routingTable().hasIndex(index))
+                                            final List<IndexResult> indices = new ArrayList<>(results.values());
+                                            final boolean acknowledged = indices.stream().noneMatch(IndexResult::hasFailures);


In closeRoutingTable, we check for:

currentState.blocks().hasIndexBlock(index.getName(), closingBlock) == false

I think this will not be matched anymore by this acknowledged check, meaning acknowledged would be true afterwards?

Also, I would think that if the block was gone and the index thus is left open, we should see a failure for it in the response?

Good observation :). I pushed 434696b to report error if closing block disappeared.

dnhatn · 2019-05-20T20:24:53Z

@henningandersen I addressed your comments. Can you please give it another look? Thank you!

henningandersen

LGTM.

henningandersen · 2019-05-22T07:23:18Z

server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponse.java


 public class CloseIndexResponse extends ShardsAcknowledgedResponse {

+    private List<IndexResult> indices;
+
    CloseIndexResponse() {
    }

    public CloseIndexResponse(final boolean acknowledged, final boolean shardsAcknowledged) {


nit: maybe we should remove this constructor to ensure it is no longer used? I personally prefer to let the caller explicitly pass the empty list, but this may be personal preference so feel free to omit.

+1. I pushed 1c9ddf0.

server/src/test/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponseTests.java

# Conflicts: # server/src/test/java/org/elasticsearch/indices/state/CloseIndexIT.java

dnhatn · 2019-05-23T18:16:29Z

@ywelsch @henningandersen Thanks for reviewing.

This changes the `CloseIndexResponse` so that it reports closing result for each index. Shard failures or exception are also reported per index, and the global acknowledgment flag is computed from the index results only. The response looks like: ``` { "acknowledged" : true, "shards_acknowledged" : true, "indices" : { "docs" : { "closed" : true } } } ``` The response reports shard failures like: ``` { "acknowledged" : false, "shards_acknowledged" : false, "indices" : { "docs-1" : { "closed" : true }, "docs-2" : { "closed" : false, "shards" : { "1" : { "failures" : [ { "shard" : 1, "index" : "docs-2", "status" : "BAD_REQUEST", "reason" : { "type" : "index_closed_exception", "reason" : "closed", "index_uuid" : "JFmQwr_aSPiZbkAH_KEF7A", "index" : "docs-2" } } ] } } }, "docs-3" : { "closed" : true } } } ``` Co-authored-by: Tanguy Leroux <[email protected]>

Relates #39687

This changes the `CloseIndexResponse` so that it reports closing result for each index. Shard failures or exception are also reported per index, and the global acknowledgment flag is computed from the index results only. The response looks like: ``` { "acknowledged" : true, "shards_acknowledged" : true, "indices" : { "docs" : { "closed" : true } } } ``` The response reports shard failures like: ``` { "acknowledged" : false, "shards_acknowledged" : false, "indices" : { "docs-1" : { "closed" : true }, "docs-2" : { "closed" : false, "shards" : { "1" : { "failures" : [ { "shard" : 1, "index" : "docs-2", "status" : "BAD_REQUEST", "reason" : { "type" : "index_closed_exception", "reason" : "closed", "index_uuid" : "JFmQwr_aSPiZbkAH_KEF7A", "index" : "docs-2" } } ] } } }, "docs-3" : { "closed" : true } } } ``` Co-authored-by: Tanguy Leroux <[email protected]>

Relates elastic#39687

The CloseIndexResponse was improved in #39687; this commit exposes it in the HLRC.

The CloseIndexResponse was improved in #39687; this commit exposes it in the HLRC. Backport of #44349 to 7.x.

Relates: #4001 Relates: elastic/elasticsearch#39687 This commit adds shards_acknowledged and indices resuls to CloseIndexResponse.

) Relates: #4001 Relates: elastic/elasticsearch#39687 This commit adds shards_acknowledged and indices resuls to CloseIndexResponse.

) Relates: #4001 Relates: elastic/elasticsearch#39687 This commit adds shards_acknowledged and indices resuls to CloseIndexResponse. (cherry picked from commit 169b784)

Close Index Response

f61462c

tlrx added >enhancement :Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. v8.0.0 labels Mar 5, 2019

tlrx requested a review from ywelsch March 5, 2019 10:01

tlrx mentioned this pull request Apr 3, 2019

Replicate closed indices #33888

Closed

50 tasks

ywelsch assigned dnhatn Apr 4, 2019

dnhatn added 2 commits April 18, 2019 14:20

extract failure explicitly

18dbcdf

Merge branch 'master' into improve-close-index-response

33f0d0d

dnhatn marked this pull request as ready for review April 18, 2019 18:37

ywelsch approved these changes Apr 30, 2019

View reviewed changes

henningandersen reviewed Apr 30, 2019

View reviewed changes

ywelsch added the v7.2.0 label May 16, 2019

dnhatn added 4 commits May 20, 2019 12:28

Merge branch 'master' into improve-close-index-response

8c4f571

shards -> shard

f6bf77e

close should fail if block disappeared

434696b

shards -> failedShards

a90aaed

dnhatn requested a review from henningandersen May 20, 2019 20:25

dnhatn added v7.3.0 and removed v7.2.0 labels May 21, 2019

henningandersen approved these changes May 22, 2019

View reviewed changes

dnhatn added 4 commits May 22, 2019 09:21

remove ctor

1c9ddf0

Merge branch 'master' into improve-close-index-response

b103916

# Conflicts: # server/src/test/java/org/elasticsearch/indices/state/CloseIndexIT.java

serialize tests

de4e32d

Merge branch 'master' into improve-close-index-response

c1806e7

dnhatn merged commit 81f3b5d into elastic:master May 23, 2019

dnhatn added the backport pending label May 23, 2019

dnhatn added a commit that referenced this pull request May 25, 2019

Adjust bwc version

1ed7b61

Relates #39687

dnhatn removed the backport pending label May 25, 2019

gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019

Adjust bwc version

65d5cb7

Relates elastic#39687

tlrx mentioned this pull request Jul 15, 2019

Add CloseIndexResponse to HLRC #44349

Merged

tlrx added a commit that referenced this pull request Jul 24, 2019

Add CloseIndexResponse to HLRC (#44349)

a69baf6

The CloseIndexResponse was improved in #39687; this commit exposes it in the HLRC.

tlrx mentioned this pull request Jul 24, 2019

[7.x] Add CloseIndexResponse to HLRC (#44349) #44788

Merged

tlrx added a commit that referenced this pull request Jul 24, 2019

[7.x] Add CloseIndexResponse to HLRC (#44349) (#44788)

a8905ef

The CloseIndexResponse was improved in #39687; this commit exposes it in the HLRC. Backport of #44349 to 7.x.

Mpdreamz mentioned this pull request Aug 7, 2019

[meta] 7.3 Release elastic/elasticsearch-net#4001

Closed

16 tasks

russcam mentioned this pull request Sep 2, 2019

Add shards_acknowledged and indices results to CloseIndexResponse elastic/elasticsearch-net#4066

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Close Index Response #39687

Improve Close Index Response #39687

tlrx commented Mar 5, 2019 •

edited by dnhatn

Loading

elasticmachine commented Mar 5, 2019

ywelsch left a comment

henningandersen left a comment

henningandersen Apr 30, 2019

dnhatn May 20, 2019

dnhatn commented May 20, 2019

henningandersen left a comment

henningandersen May 22, 2019

dnhatn May 22, 2019

dnhatn commented May 23, 2019

Improve Close Index Response #39687

Improve Close Index Response #39687

Conversation

tlrx commented Mar 5, 2019 • edited by dnhatn Loading

elasticmachine commented Mar 5, 2019

ywelsch left a comment

Choose a reason for hiding this comment

henningandersen left a comment

Choose a reason for hiding this comment

henningandersen Apr 30, 2019

Choose a reason for hiding this comment

dnhatn May 20, 2019

Choose a reason for hiding this comment

dnhatn commented May 20, 2019

henningandersen left a comment

Choose a reason for hiding this comment

henningandersen May 22, 2019

Choose a reason for hiding this comment

dnhatn May 22, 2019

Choose a reason for hiding this comment

dnhatn commented May 23, 2019

tlrx commented Mar 5, 2019 •

edited by dnhatn

Loading