ILM use Priority.IMMEDIATE for stop ILM cluster update

[andreidan]

This changes the priority of the cluster state update that stops ILM
altogether to IMMEDIATE. We've chosen to change this as it can be useful to
temporarily stop ILM if a cluster is overwhelmed, but a NORMAL
priority can see the "stop ilm update" not make it up the tasks queue.

On the same note, we're keeping the start ILM cluster update priority
to NORMAL on purpose such that we only start ILM if the cluster can
handle it.

[elasticmachine]

Pinging @elastic/es-core-features (:Core/Features/ILM+SLM)

[andreidan]

@elasticmachine update branch

[dakrone]

Thanks @andreidan, this needs to update the second half of the shutdown to also use the priority though, so both parts are urgent

[DaveCTurner]

I suggest IMMEDIATE rather than URGENT for this task. I don't know of any starvation issues at URGENT priority but my concern is that we may uncover such an issue in future which prevents us from stopping ILM in a timely fashion. This action is very lightweight, only available to admins, rarely called, but useful when the cluster is under severe pressure, and I can't think of a reason for spamming it that isn't obviously abusive.

Please sprinkle some comments throughout these cluster state updates indicating that they may be running at IMMEDIATE priority, as a defence against accidentally making them more heavyweight in future.

[DaveCTurner]

Also FWIW you can test that the task really runs at a high enough priority with something like this:

elasticsearch/server/src/test/java/org/elasticsearch/cluster/ClusterHealthIT.java

Lines 268 to 287 in 95a7eed

	final AtomicBoolean keepSubmittingTasks = new AtomicBoolean(true);
	final ClusterService clusterService = internalCluster().getInstance(ClusterService.class, internalCluster().getMasterName());
	clusterService.submitStateUpdateTask("looping task", new ClusterStateUpdateTask(Priority.LOW) {
	@Override
	public ClusterState execute(ClusterState currentState) {
	return currentState;
	}
	@Override
	public void onFailure(String source, Exception e) {
	throw new AssertionError(source, e);
	}
	@Override
	public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) {
	if (keepSubmittingTasks.get()) {
	clusterService.submitStateUpdateTask("looping task", this);
	}
	}
	});

(replace Priority.LOW with Priority.IMMEDIATE) -- even if the cluster is processing this looping task it should still process other IMMEDIATE tasks but nothing lower. I prefer that to simply asserting that it returns the right .priority().

[andreidan]

@DaveCTurner thanks for pointing out the ClusterHealthIT. I wasn't aware of it.

If you don't have a strong opinion on adding an integration test, I'd rather not, as ILM is a client of the ClusterService, using the ClusterService API according to the specification (I think ensuring the order and fairness of task execution is something that we should, and probably are, test as part of the cluster module - I think the PrioritizedExecutorsTests is making sure the executor invariants hold)

[dakrone]

LGTM, thanks Andrei