Add `explain` flag support to the reroute API #5027

dakrone · 2014-02-05T20:44:09Z

By specifying the explain flag, an explanation for the reason a
command can or cannot be executed is returned. No allocation commands
are actually performed.

Returns a response similar to:

{
  "state": { ... cluster state ...},
  "acknowledge": true,
  "explanations" : [ {
    "command" : "cancel",
      "parameters" : {
        "index" : "decide",
        "shard" : 0,
        "node" : "IvpoKRdtRiGrQ_WKtt4_4w",
        "allow_primary" : false
      },
      "decisions" : [ {
        "decider" : "cancel_allocation_command",
        "decision" : "YES",
        "explanation" : "..."
        } ]
     }, {
      "command" : "move",
      "parameters" : {
        "index" : "decide",
        "shard" : 0,
        "from_node" : "IvpoKRdtRiGrQ_WKtt4_4w",
        "to_node" : "IvpoKRdtRiGrQ_WKtt4_4w"
       },
       "decisions" : [ {
         "decider" : "same_shard",
         "decision" : "NO",
         "explanation" : "shard cannot be allocated on same node [IvpoKRdtRiGrQ_WKtt4_4w] it already exists on"
       },
       etc, etc, etc
       ]
  }]
}

An example of the full output is in #2483

Closes #5169

s1monw · 2014-02-06T08:22:36Z

src/main/java/org/elasticsearch/action/admin/cluster/reroute/TransportClusterRerouteAction.java

-                clusterStateToSend = newState;
-                if (request.dryRun) {
-                    return currentState;
+                if (request.explain()) {


so it is not possible to get the explain if I also want to reroute ie. actually exectue the operation? I mean we could still make ups of the dryRun flag and get explains if dryRun = false

By specifying the `explain` flag, an explanation for the reason a command can or cannot be executed is returned. No allocation commands are actually performed. Returns a response similar to: { "explanations" : [ { "command" : "cancel", "parameters" : { "index" : "decide", "shard" : 0, "node" : "IvpoKRdtRiGrQ_WKtt4_4w", "allow_primary" : false }, "decisions" : [ { "decider" : "CancelAllocationCommand", "decision" : "YES", "explanation" : "..." } ] }, { "command" : "move", "parameters" : { "index" : "decide", "shard" : 0, "from_node" : "IvpoKRdtRiGrQ_WKtt4_4w", "to_node" : "IvpoKRdtRiGrQ_WKtt4_4w" }, "decisions" : [ { "decider" : "same_shard", "decision" : "NO", "explanation" : "shard cannot be allocated on same node [IvpoKRdtRiGrQ_WKtt4_4w] it already exists on" }, etc ] }] } Closes elastic#2483

…ation

…ision

s1monw · 2014-02-19T09:10:28Z

src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteRequest.java

@@ -110,6 +128,7 @@ public void readFrom(StreamInput in) throws IOException {
        super.readFrom(in);
        commands = AllocationCommands.readFrom(in);
        dryRun = in.readBoolean();
+        explain = in.readBoolean();


I guess we need a version check here sicne we are changing the protocol?

s1monw · 2014-02-19T09:36:28Z

I have a couple of problems with how this at the current stage. Implementation wise the code looks great but I think from my understanding a reroute call with explain = true implies dryRun=true no matter what you set there. Is that correct? If so I really thing we shouldn't do this. It will confuse the hell out of people if we treat this not as a flag but as a different action. I personally thing we should make explain the default and execute it all the time. so basically we should only have a flag for dryRun and always build an explain. if folks are not interested in the response they can just ignore it. If you want to simulate you set dryRun=true and you always see what is going on, makes sense?

dakrone · 2014-02-19T14:23:58Z

Yea, I understand what you're saying. I will work on removing the RoutingAllocation.Result from the cluster state, which should allow me to return the explain results every time, even while executing the commands.

dakrone · 2014-02-21T18:07:55Z

@s1monw okay, I have made a lot of changes, please take another look

removed AllocationExplanation from cluster state, as per remove RoutingAllocation.Result from cluster state #5169 (the RoutingTable is still part of the state, which is why the entire Result object is not removed)
made the RoutingExplanations part of the Result object now
you can now do .explain(true) and execute the reroute commands in addition to explaining (or still do a dry_run with explanations)
removed the separate .explain() methods on the allocation commands to removed the duplicated code
added version checks for the changed serialization in the Reroute stuff and the ClusterState

s1monw · 2014-02-26T21:26:16Z

src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteRequest.java

@@ -110,6 +129,11 @@ public void readFrom(StreamInput in) throws IOException {
        super.readFrom(in);
        commands = AllocationCommands.readFrom(in);
        dryRun = in.readBoolean();
+        if (in.getVersion().onOrAfter(Version.V_2_0_0)) {


I think this can be V_1_1_0?

Yea, I wasn't sure where this was going to end up, thought it might be 2.0-only since it includes cluster state changes, but I can definitely change that.

s1monw · 2014-02-26T21:39:06Z

I like this much better I left some comments I think we are close...

dakrone · 2014-02-26T22:05:09Z

@s1monw updated again with the changes you mentioned.

s1monw · 2014-02-27T11:16:16Z

LGTM

dakrone · 2014-02-27T17:31:17Z

Merged to 1.x and master

s1monw reviewed Feb 6, 2014
View reviewed changes

dakrone added 3 commits February 18, 2014 10:35

Move explain endpoint to /_cluster/reroute/explain, add more document…

56be797

…ation

Fix issue with CancelAllocationCommand incorrectly reporting a NO dec…

973a72c

…ision

s1monw reviewed Feb 19, 2014
View reviewed changes

dakrone added 2 commits February 20, 2014 12:12

Remove AllocationExplanation from cluster state

35f6c0b

first cut of explanation while executing reroute command

17357ef

dakrone added 3 commits February 21, 2014 10:58

fix javadoc

96c91cb

Remove now-unneeded explain() method in AllocationService

c6dcc5d

clean up imports in ClusterRerouteTests

a694642

s1monw reviewed Feb 26, 2014
View reviewed changes

dakrone added 5 commits February 26, 2014 14:43

send correct state if nodes timeout

dfbeb61

add missing [] around node

eeb55a4

assert that explanations are not null

1c8f247

Target 1.1.0 instead of 2.0.0

9b4938c

Randomly set explain parameter for reroute tests

7adbf37

dakrone closed this Feb 27, 2014

dakrone deleted the explain-reroute branch February 27, 2014 20:18

clintongormley added >enhancement v1.1.0 v2.0.0-beta1 :Core/Infra/REST API REST infrastructure and utilities labels Jun 7, 2015

cywjackson mentioned this pull request May 22, 2016

unassigned replica shards after cluster restart #9602

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `explain` flag support to the reroute API #5027

Add `explain` flag support to the reroute API #5027

dakrone commented Feb 5, 2014

s1monw Feb 6, 2014

s1monw Feb 19, 2014

s1monw commented Feb 19, 2014

dakrone commented Feb 19, 2014

dakrone commented Feb 21, 2014

s1monw Feb 26, 2014

dakrone Feb 26, 2014

s1monw commented Feb 26, 2014

dakrone commented Feb 26, 2014

s1monw commented Feb 27, 2014

dakrone commented Feb 27, 2014

Add explain flag support to the reroute API #5027

Add explain flag support to the reroute API #5027

Conversation

dakrone commented Feb 5, 2014

s1monw Feb 6, 2014

Choose a reason for hiding this comment

s1monw Feb 19, 2014

Choose a reason for hiding this comment

s1monw commented Feb 19, 2014

dakrone commented Feb 19, 2014

dakrone commented Feb 21, 2014

s1monw Feb 26, 2014

Choose a reason for hiding this comment

dakrone Feb 26, 2014

Choose a reason for hiding this comment

s1monw commented Feb 26, 2014

dakrone commented Feb 26, 2014

s1monw commented Feb 27, 2014

dakrone commented Feb 27, 2014

Add `explain` flag support to the reroute API #5027

Add `explain` flag support to the reroute API #5027