Create IndexVersionAllocationDecider

[thecoop]

Create IndexVersionAllocationDecider alongside NodeVersionAllocationDecider. This uses IndexVersion to ensure we don't allocate to nodes that have different IndexVersions (but might have the same Version). Eventually, NodeVersionAllocationDecider will go away, but we're not there yet

[elasticsearchmachine]

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

[DaveCTurner]

This allocation decider exists because we need to be able to copy data (both segments and operations) from the primary to the replica, which means that the replica must understand at least everything that the primary understands. However @rjernst just mentioned to me that IndexVersion doesn't cover certain changes that would prevent us from copying data from a newer version to an older version, such as the introduction of new mapping types. I think we need to account for all sorts of compatibility issues in this decider.

[thecoop]

I've modified this to add IndexVersionAllocationDecider alongside NodeVersionAllocationDecider. This will improve the behaviour on serverless, but as discussed we need more work before we can replace NodeVersionAllocationDecider completely.

[thecoop]

@DaveCTurner Could we get this in alongside NodeVersionAllocationDecider? It will prevent allocation problems when IndexVersion is bumped on serverless. We can remove VersionAllocationDecider later when we find a solution to the mapping type/option allocation problem

[thecoop]

@elasticmachine update branch

[thecoop]

@DaveCTurner I've updated this to use the mapping update index version added by #106748.

Do we also need to consider the highest index version used to write a shard, as added by #106779? Or if the mappings aren't updated, does it matter? If a new shard is created as part of a write, will have the index version of the node that created it?

[thecoop]

So, IndexVersionAllocationDeciderTests.testRollingRestart is failing, however I'm not sure this test is valid as-is. This test is ensuring that indicies do not get moved from a higher version to a lower version node during a restart...but we're not longer considering the node versions we're moving to/from, we're now looking at the index metadata and what is actually stored.

Maybe combined with #106779, might it actually be valid to move a shard from a later version node to an earlier node, if nothing has actually changed about the shard?

I've also removed the distinction between primary and replica shard behaviour, now its just considering the shard as-is. I don't think there's a difference needed between the two

[henningandersen]

Left a few initial comments to the decider.

[henningandersen]

Should this use isIndexVersionCompatibleRelocatePrimary as well?

[henningandersen]

I wonder if this should check for same or newer index version of the primary vs replica nodes in addition to the check against metadata?

[thecoop]

I think we also need a data version check - make sure we don't allocate shards to an index version that is less than the highest index version used to write data. This needs to be stored in index metadata, which means we need some kind of listener to be notified of new commits being added, and which index version it is, and stores that version in the index metadata.

[thecoop]

As discussed, I have re-created IndexVersionAllocationDecider by copying NodeVersionAllocationDecider, replacing references to node version with maximum index version

[henningandersen]

Looks good. Have a few comments, primarily on the test.

[henningandersen]

Should we use a newer node-version here to check that the index version decider is the one that prevents the allocation? And perhaps vice-versa in NodeVersionAllocationDeciderTests?

[thecoop]

Updated. The one exception is deciding on snapshot allocations - which can only use index version. NodeVersionAllocationDecider references IndexVersion in isVersionCompatible - is it best to just remove that path so NodeVersionAllocationDecider always returns ALWAYS for snapshots, and leaves it up to IndexVersionAllocationDecider?

[henningandersen]

I'd leave that as is for this PR. To some extent, NodeVersionAllocationDecider ought to use the node-version until we are certain that the index-version will work, but that is umodified by this.

[henningandersen]

LGTM.