Fork post-snapshot-delete cleanup off master thread

[DaveCTurner]

We shouldn't run the post-snapshot-delete cleanup work on the master
thread, since it can be quite expensive and need not block subsequent
cluster state updates. This commit forks it onto a SNAPSHOT thread.

[elasticsearchmachine]

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

[elasticsearchmachine]

Hi @DaveCTurner, I've created a changelog YAML for you.

[DaveCTurner]

Rework of #122047 with a test fix to ensure the repository can be unregistered during test cleanup - see #122227 and linked test failures.

[nicktindall]

LGTM

[nicktindall]

Nice, I had a look at this as well when the original problems popped up. I investigated adding a

  - do:
      tasks.list:
        actions: cluster:admin/snapshot/delete
        wait_for_completion: true

to the end of the test, but looking at the code, the task completing didn't necessarily mean the delete had occurred?

[mhl-b]

I think it does. My understanding that deletion request goes into snapshotDeletionListeners and proceed with deletion, update cluster state when deletion is complete, and notify snapshotDeletionListeners on cluster state update. I guess it comes to the repository implementation, if delete calls are synchronous, otherwise we need to pull state...

[DaveCTurner]

In the ?wait_for_completion=false case we don't register the listener with snapshotDeletionListeners:

elasticsearch/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java

Lines 2227 to 2231 in aac1409

	if (newDelete == null || request.waitForCompletion() == false) {
	listener.onResponse(null);
	} else {
	addDeleteListener(newDelete.uuid(), listener);
	}

[mhl-b]

LGTM

[elasticsearchmachine]

💚 Backport successful

Status	Branch	Result
✅	8.18
✅	8.x
✅	9.0