Fix cluster settings update task acknowledgment #97111
Conversation
tlrx added >bug :Distributed Coordination/Cluster Coordination 
| @Override | ||
| public void onAckFailure(Exception e) { | ||
| if (changed) { | ||
| reroute(true); |
| Hi @tlrx, I've created a changelog YAML for you. |
| Pinging @elastic/es-distributed (Team:Distributed) |
elasticsearchmachine added the Team:Distributed (Obsolete) 
There was a problem hiding this comment.
Looks ok but it would be nice to validate this change with a test too.
Moreover this is very much an old-style unbatched cluster state update which passes the execution result via mutable state held by the task. Batchifying it would make it easier to test I think.
| @Override | ||
| public void onAckFailure(Exception e) { | ||
| if (changed) { | ||
| reroute(true); |
...va/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java Show resolved Hide resolved
It's a trap! 😁 Although I would like to see this action batchified, it's actually kinda complex. Let's not let that work stand in the way of this bug fix. |
…-not-acked' into do-not-ack-update-settings-if-task-not-acked
| @DaveCTurner thanks for your comments. I added a test for the specific bug fix. I'm not super happy with it but I haven't been able to come with something less hacky. Let me know what you think. |
| Thanks David |
ClusterUpdateSettingsTask uses a "changed" variable to set the value of the acknowledged flag of the ClusterUpdateSettingsResponse that is returned to the client. The "changed" variable is computed from the cluster state update but there is no guarantee that the update will be acknowledged. In case the update failed to be acked (onAckFailure) it should return the flag appropriately.
💚 Backport successful
|
ClusterUpdateSettingsTask uses a "changed" variable to set the value of the acknowledged flag of the ClusterUpdateSettingsResponse that is returned to the client. The "changed" variable is computed from the cluster state update but there is no guarantee that the update will be acknowledged. In case the update failed to be acked (onAckFailure) it should return the flag appropriately.
4 participants
ClusterUpdateSettingsTaskuses achangedvariable to set the value of theacknowledgedflag of theClusterUpdateSettingsResponsethat is returned to the client.The
changedvariable is computed from the cluster state update but there is no guarantee that the update will be acknowledged. In case the update failed to be acked (onAckFailure) it should return the flag appropriately.