Auto-expand replicas when adding or removing nodes

[ywelsch]

Auto-expands replicas in the same cluster state update (instead of a follow-up reroute) where nodes are added or removed.

Fixes #1873

[elasticmachine]

Pinging @elastic/es-distributed

[bleskes]

LGTM. +1 to move it into the allocation service. I left some nits and questions. Thanks for picking this up!

[bleskes]

I'm confused by this . If min<= max, we always match this clause?

[ywelsch]

yeah, if (only) min <= max. Unfortunately we never validated this condition anywhere (I would have expected this during setting validation, but to my surprise, it was not there), and I didn't feel like making a breaking change in this PR by strengthening the validation logic. For now, just as before, if min > max, the auto-expand replicas will be ignored.

[bleskes]

can we add some java docs about that we're computing here?

[bleskes]

I know you copied over how it was, so this is optional in my mind as it is out of scope of the PR - I'm personally find this confusing without much added value and would prefer a String -> Integer map which gives a per index updated value.

[ywelsch]

The reason I chose to stick with this model for now is that the PR still calls RoutingTable.Builder#updateNumberOfReplicas(int numberOfReplicas, String... indices) to make the actual update, which is also used by MetaDataUpdateSettingsService when a user updates the number of replicas manually. This allows to reuse the method as is. As a follow-up, I would like us to migrate updateNumberOfReplicas from RoutingTable.Builder to RoutingNodes, as I would like (for consistency purposes) all routing table updates to go through that class instead.
I've added some javadocs to explain this.

[ywelsch]

Thanks @bleskes

[lukas-vlcek]

@ywelsch I was quickly looking at the test in this PR. This issue resulted in dropping the data and resyncing it from other nodes right after that. If I read the test correctly it works only with cluster state a bit, it does not verify that the data resyncing issue does not occur explicitly. So the assumption is that changes in how the cluster state is represented now should not lead to the replication data issue, is that correct?

[ywelsch]

@lukas-vlcek The issue resulted from how two distinct components in the system interacted: the auto-expand replica logic, and the shard deletion logic. The way the shard deletion logic works is as follows: When a data node receives a cluster state where a shard is fully allocated (i.e. primary + all replicas are active) and that node has some unallocated data for that shard on disk, it proceeds to delete said data as there is no need to keep this extra copy of the data around. The way the auto-expand replica logic used to work was as follows: This component hooked into the cluster lifecycle, waiting for cluster state changes, comparing the number of nodes in the cluster with the number of auto-expanded replicas and then submitting a settings update to the cluster to adjust the number_of_replicas if the two got out of sync. The way these two components now interacted resulted in the following behavior: Assume a 5 node cluster and an index with auto-expand-replicas set to 0-all. In that case, number_of_replicas is expanded to 4 (i.e. there are 4 replica copies for each primary). Now assume that one node drops out only to later on rejoin the cluster. When the node drops from the cluster, it is removed from the cluster state. In a follow-up settings update initiated by the auto-expand-logic, number_of_replicas for the index is adjusted from 4 to 3. When the node rejoins the cluster, it is added to the cluster state by the master and receives this cluster state. The cluster state now has 5 nodes again, but the number_of_replicas is not expanded yet from 3 to 4, as this is only triggered shortly after by the auto-expand component (running on the active master). The shard deletion logic on the data node now sees that the shard is fully allocated (number_of_replicas set to 3 with all shard copies active). It then proceeds to delete the extra local shard copy, just before it receives the updated cluster state where number_of_replicas is now adjusted to 4 again. It therefore has to resync the full data from the primary again as the local copy was completely wiped.

The two main options to fix this were:

• make the shard deletion logic auto-expand replicas aware, accounting for the inconsistencies in the cluster state.
• ensure that cluster states are always consistent w.r.t the auto-expansion of replicas.

For better separation of concerns, and to have stronger consistency guarantees on cluster states, I opted for the second kind of fix. With this PR, the auto-expansion of replicas is done in the same cluster state update where nodes are added or removed from the cluster state. This means that when the newly joining node receives its first cluster state, number_of_replicas is already correctly expanded to 4, and the node is aware that the shard is not fully allocated, and therefore does not proceed to delete its local shard copy. The tests in this PR check that the consistency guarantees in the cluster state (number_of_replicas is properly expanded at all times) cannot be violated. The shard deletion logic is already separately tested, working on the assumption of a consistent cluster state.