Track evacuated IDs since the last shard-local refresh in LiveVersionMap

[pxsalehi]

This PR adds adds an interface and noop implementation for LiveVersionMapArchive.
It receives old maps from the LiveVersionMap upon a refresh, and is also informed
when unpromotable shards have been refreshed (one way of doing this is implemented
in the matching Serverless PR).

relates ES-5728

[pxsalehi]

For now please ignore correctness/concurrency issues. I'm wondering if I could create and pass around UnpromotableRefresher entirely from within the stateless plugin.

[Tim-Brooks]

I'm wondering if I could create and pass around UnpromotableRefresher entirely from within the stateless plugin.

You can create overridable methods that only IndexEngine implements.

I added this PR as a discussion point for the sync tomorrow. I spent some time looking at it, but still thinking through the approach. I think we should all discuss it.

[pxsalehi]

You can create overridable methods that only IndexEngine implements.

I meant more how to wire-up and pass along an UnpromotableRefresher that is NOT a no-op, entirely from Stateless.java. It would need to be able to send UnpromotableShardRefreshRequests and have access to the IndexShard. I think for that we need to:

• Make TransportUnpromotableShardRefreshAction an action that can be called using a NodeClient.
• Create the stateless UnpromotableRefresher implementation and pass it to IndexEngine by giving it the required IndexShard using an IndexEventListener maybe.

At the moment this is not urgent but I think if possible, this would cleanup a lot of noise here and move it to the Serverless PR where these things are actually used/needed.

[elasticsearchmachine]

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

[henningandersen]

Left a few comments after an initial skimming of central pieces.

[pxsalehi]

@henningandersen @tlrx This is now ready for review again. I've removed most of the clutter.

[henningandersen]

This looks much leaner, I am largely on board but have not gone in depth yet. I have some first comments about the structure and interaction between server and stateless (also in the companion PR).

[henningandersen]

One more comment around safety.

[pxsalehi]

@henningandersen I've addressed the clean ups you suggested. Please take another look.

[henningandersen]

This looks good to me. I wonder if we can add some minimal testing of the archive interaction in :server: ? Perhaps something like:

1. Validate that an override of createLiveVersionMapArchiver results in a LiveVersionMap with the archive in it.
2. Validate that beforeRefresh does not call it and afterRefresh does call it.
3. Validate that an archive with some value in it is found when using the live-version map.

[pxsalehi]

@henningandersen I've addressed the changes.

I wonder if we can add some minimal testing of the archive interaction in :server: ?

hmm... I find that a bit of an overkill. I looked into that a bit and I think it would probably be a test with a bunch of awkward setups and trivial assertions. If you think that would be crucial for the PR, I can do that. I've tested all of those things in the serverless PR, where I think it actually matters.

[henningandersen]

@henningandersen I've addressed the changes.

I wonder if we can add some minimal testing of the archive interaction in :server: ?

hmm... I find that a bit of an overkill. I looked into that a bit and I think it would probably be a test with a bunch of awkward setups and trivial assertions. If you think that would be crucial for the PR, I can do that. I've tested all of those things in the serverless PR, where I think it actually matters.

I'd like to have it. I think an important mechanism like this deserves some basic testing here. The effort does not sound enormous to me. I get that there is limited value, but not breaking the contract and knowing that early is beneficial. And I fear that the large separation of test from the code here will cause them to not be updated in concert moving forward. If some of it requires big efforts I am happy to omit it, but can you give it a shot and we can chat about it if you think it is too big?

[pxsalehi]

@henningandersen I've added the requested tests, please have another look!

[henningandersen]

LGTM.