Add utility for tracking licensed persistent tasks

[rjernst]

Licensed feature tracking of long running features happens now by
starting and stopping the tracking of a feature. This commit adds an
intermediate base class to be used by features that utilize persistent
tasks. The base class only requires specifying the feature and context,
and then any tasks created automatically are tracked. An example using
marchine learning's JobTask is also added here, using a new machine
learning job feature object.

[elasticmachine]

Pinging @elastic/es-security (Team:Security)

[benwtrent]

Its really nice how simple this is. Integration with it will be 🍰

I have some comments around inheritance safety + weird exception paths.

[benwtrent]

I think these should all be final and there should be new methods called doMarkAs... or something.

There is a nasty little tendency for persistent tasks to accidentally overwrite a method and not call super. This has caused non-trivial to debug bugs in the past.

[benwtrent]

Suggested change

	LicensedFeature.persistent("machine-learning-job", License.OperationMode.PLATINUM);
	LicensedFeature.persistent("machine-learning-anomaly-detection-job", License.OperationMode.PLATINUM);

Since we also have data-frame-analytics-job :D

[benwtrent]

Looking at the taskManager code, I think there is a small chance that a task object is destroyed but none of the cancellations methods are called.

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

Lines 178 to 202 in 7d980ad

	try {
	task.init(persistentTasksService, taskManager, taskInProgress.getId(), taskInProgress.getAllocationId());
	logger.trace("Persistent task [{}] with id [{}] and allocation id [{}] was created", task.getAction(),
	task.getPersistentTaskId(), task.getAllocationId());
	try {
	runningTasks.put(taskInProgress.getAllocationId(), task);
	nodePersistentTasksExecutor.executeTask(taskInProgress.getParams(), taskInProgress.getState(), task, executor);
	} catch (Exception e) {
	// Submit task failure
	task.markAsFailed(e);
	}
	processed = true;
	} catch (Exception e) {
	initializationException = e;
	} finally {
	if (processed == false) {
	// something went wrong - unregistering task
	logger.warn("Persistent task [{}] with id [{}] and allocation id [{}] failed to create", task.getAction(),
	task.getPersistentTaskId(), task.getAllocationId());
	taskManager.unregister(task);
	if (initializationException != null) {
	notifyMasterOfFailedTask(taskInProgress, initializationException);
	}
	}
	}

Note if init is called and throws, only taskManager.unregister(task); is called. I don't think this calls any of these internal life time tracking methods.

I am thinking that licensedFeature.startTracking(licenseState, featureContext); should probably be called in init to avoid this weird condition.

What do you think?

[rjernst]

It seems like init is being abused currently. It's purpose, IMO is to just set some references, not to do any work? But at least rollup seems to utilize this.

What if instead I were to make init public and final? Public is so that tests can still call it, and then final so that there is no chance some extra work is done that could actually throw an exception.

[benwtrent]

@rjernst if we can make that guarantee of it not throwing, it makes sense to me. All it's doing is setting internal values...so I don't know why it would ever throw.

So, I think fix init, or somehow make sure that stopTracking is called if init throws in this execution path.

[rjernst]

@elasticmachine run elasticsearch-ci/part-2-fips

[rjernst]

@benwtrent This is ready for another look.

[benwtrent]

👍