Add new setting to disable persistent tasks allocations #29137

tlrx · 2018-03-19T14:22:45Z

This commit adds a new setting cluster.persistent_tasks.allocation.enable that can be used to enable or disable the allocation of persistent tasks.

The setting accepts the values all (default) or none. When set to none, the persistent tasks that are created (or that must be reassigned) won't be assigned to a node but will reside in the cluster state with
a null executor node and a reason describing why it is not assigned:

"assignment" : {
  "executor_node" : null,
  "explanation" : "persistent task [foo/bar] cannot be assigned [no persistent task assignments are allowed due to cluster settings]"
}

This commit adds some AssignmentDecider and AssignmentDecision classes that are similar to the deciders/decisions classes used for shard allocations. It also adds an EnableAssignmentDecider decider that is responsible for registering the new cluster setting.

The setting is checked when executing the PersistentTasksClusterService#createAssignment() method, before delegating the creation of the assignment to the PersistentTasksExecutor. As a follow up, I think that PersistentTasksExecutor could expose a getTaskAllocationDecider method that would be used by the persistent task cluster service to build a list of nodes that could execute the new task. Then this list of assignable nodes could be passed again to the executor to select the exact node to execute the task.

elasticmachine · 2018-03-19T14:22:47Z

Pinging @elastic/es-distributed

imotov

Could you open an issue for the follow-up that you proposed at the end of your PR so it wouldn't get lost? Without such follow-up this PR doesn't make a lot of sense. Ideally, I would have preferred for this follow-up to happen first and this change to follow, but it might be not possible if we need this for 6.3.

imotov · 2018-03-19T16:00:52Z

server/src/main/java/org/elasticsearch/persistent/decider/AssignmentDecision.java

+ */
+public final class AssignmentDecision {
+
+    public static final AssignmentDecision ALWAYS = new AssignmentDecision(Type.YES);


That feels like premature optimization.

+1 . People can just use yes? I'm not sure what the difference is?

Right, it's not required.

tlrx · 2018-03-19T17:34:19Z

Could you open an issue for the follow-up that you proposed at the end of your PR so it wouldn't get lost? Without such follow-up this PR doesn't make a lot of sense. Ideally, I would have preferred for this follow-up to happen first and this change to follow, but it might be not possible if we need this for 6.3.

Yes, the follow up is a bigger change and we'd like the setting to be added in 6.3, thus the order of pull requests. Thanks for your review Igor.

bleskes

Thx @tlrx . I left some suggestions, mostly about removing stuff that are not yet needed.

I would love to have an integration test (i.e., qa) to check that what we need (setting this in the es.yml file actually prevents initial allocation of tasks and that you can enable them from the API). This may be tricky so we need to check whether we can do this in time.

Re your suggestion of opening up the deciders to the executors - I'll reach you to talk in another channel. I want to understand better where you see this heading

bleskes · 2018-03-21T10:02:53Z

server/src/main/java/org/elasticsearch/persistent/PersistentTasksClusterService.java

@@ -45,12 +48,14 @@

    private final ClusterService clusterService;
    private final PersistentTasksExecutorRegistry registry;
+    private final AssignmentDecider<PersistentTaskParams> decider;


can we just hard code this for now to EnableAssignmentDecider? we can generalize when it becomes relevant (and know what we want exactly)

bleskes · 2018-03-21T10:07:43Z

server/src/main/java/org/elasticsearch/persistent/decider/AssignmentDecider.java

+ * {@link AssignmentDecider} are used to decide if a persistent
+ * task can be assigned to a node.
+ */
+public abstract class AssignmentDecider<Params extends PersistentTaskParams> extends AbstractComponent {


I would prefer this to be added at the same time we support multiple deciders.

bleskes · 2018-03-21T10:11:20Z

server/src/main/java/org/elasticsearch/persistent/decider/AssignmentDecision.java

+ */
+public final class AssignmentDecision {
+
+    public static final AssignmentDecision ALWAYS = new AssignmentDecision(Type.YES);


+1 . People can just use yes? I'm not sure what the difference is?

bleskes · 2018-03-21T10:11:40Z

server/src/main/java/org/elasticsearch/persistent/decider/AssignmentDecision.java

+
+    public static final AssignmentDecision ALWAYS = new AssignmentDecision(Type.YES);
+    public static final AssignmentDecision YES = new AssignmentDecision(Type.YES);
+    public static final AssignmentDecision NO = new AssignmentDecision(Type.NO);


I'm not sure we want this? it means people can say no without giving a reason.

bleskes · 2018-03-21T10:12:26Z

server/src/main/java/org/elasticsearch/persistent/decider/EnableAssignmentDecider.java

+ *
+ * @see Allocation
+ */
+public class EnableAssignmentDecider<Params extends PersistentTaskParams> extends AssignmentDecider<Params> {


Why is this generic?

This PR removes the method getAssignment() for the PersistentTasksExecutor in favor of `getTaskAssignmentDecider()` that allows the executor to declare the AssignmentDecider to use when assigning the tasks. This decider is combined with the built-in ones in order to allow the PersistentTasksClusterService to decide if a task can be assigned or not, and on which nodes. Then, the final choice of the assigned node is let to the executor that can apply its custom logic. The goal is to allow top level decisions for persistent tasks assignment in the PersistentTasksClusterService (see elastic#29137) while splitting the logic of task assignment in smaller chunks into dedicated deciders, because persistent tasks use some similar rules (like "assign to data node only", "node must be in version X" etc) that can be shared.

tlrx · 2018-03-21T14:12:25Z

Thanks @imotov and @bleskes and sorry for all the premature stuff.

@bleskes I addressed your comments and also added an integration test (but not a QA one), please let me know what you think.

bleskes

LGTM. Thanks @tlrx

This commit adds a new setting `cluster.persistent_tasks.allocation.enable` that can be used to enable or disable the allocation of persistent tasks. The setting accepts the values `all` (default) or `none`. When set to none, the persistent tasks that are created (or that must be reassigned) won't be assigned to a node but will reside in the cluster state with a no "executor node" and a reason describing why it is not assigned: ``` "assignment" : { "executor_node" : null, "explanation" : "persistent task [foo/bar] cannot be assigned [no persistent task assignments are allowed due to cluster settings]" } ``` This commit adds some AssignmentDecider and AssignmentDecision classes that are similar to the deciders/decisions classes used for shard allocations. It also adds an EnableAssignmentDecider decider that is responsible for registering the new cluster settings. The setting is checked when executing the PersistentTasksClusterService `createAssignment()` method, before delegating the creation of the assignment to the PersistentTasksExecutor. As a follow up, I think that PersistentTasksExecutor could expose a `getTaskAllocationDecider` method that would be used by the PersistentTasksClusterService to build a list of nodes that could execute the new task. Then this list of assignable nodes could be passed again to the task executor to select the exact node to execute the task.

This commit adds a new setting `cluster.persistent_tasks.allocation.enable` that can be used to enable or disable the allocation of persistent tasks. The setting accepts the values `all` (default) or `none`. When set to none, the persistent tasks that are created (or that must be reassigned) won't be assigned to a node but will reside in the cluster state with a no "executor node" and a reason describing why it is not assigned: ``` "assignment" : { "executor_node" : null, "explanation" : "persistent task [foo/bar] cannot be assigned [no persistent task assignments are allowed due to cluster settings]" } ```

tlrx · 2018-03-22T08:32:41Z

Thanks @bleskes

tlrx added review :Distributed/Task Management Issues for anything around the Tasks API - both persistent and node level. v7.0.0 v6.3.0 labels Mar 19, 2018

tlrx requested review from imotov and bleskes March 19, 2018 14:22

imotov approved these changes Mar 19, 2018

View reviewed changes

bleskes suggested changes Mar 21, 2018

View reviewed changes

bleskes approved these changes Mar 21, 2018

View reviewed changes

tlrx added 4 commits March 21, 2018 20:32

Fix license header

8d91188

Apply feedback

69d93ac

Fix test

3648a2e

tlrx force-pushed the assignment-deciders branch from 77db006 to 3648a2e Compare March 21, 2018 19:35

tlrx merged commit edf27a5 into elastic:master Mar 22, 2018

elasticmachine mentioned this pull request Apr 25, 2018

[ML] Add option to allow jobs to be opened when there are no ML nodes #29991

Closed

jpountz added the >feature label Jun 13, 2018

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new setting to disable persistent tasks allocations #29137

Add new setting to disable persistent tasks allocations #29137

tlrx commented Mar 19, 2018

elasticmachine commented Mar 19, 2018

imotov left a comment

imotov Mar 19, 2018

bleskes Mar 21, 2018

tlrx Mar 21, 2018

tlrx commented Mar 19, 2018

bleskes left a comment

bleskes Mar 21, 2018

bleskes Mar 21, 2018

bleskes Mar 21, 2018

bleskes Mar 21, 2018

tlrx Mar 21, 2018

bleskes Mar 21, 2018

tlrx commented Mar 21, 2018

bleskes left a comment

tlrx commented Mar 22, 2018

Add new setting to disable persistent tasks allocations #29137

Add new setting to disable persistent tasks allocations #29137

Conversation

tlrx commented Mar 19, 2018

elasticmachine commented Mar 19, 2018

imotov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlrx commented Mar 19, 2018

bleskes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlrx commented Mar 21, 2018

bleskes left a comment

Choose a reason for hiding this comment

tlrx commented Mar 22, 2018