fix a few versionAdded values in ElasticsearchExceptions

[talevy]

TooManyBucketsException was introduced in v6.2
and SnapshotInProgressException was introduced in v6.7

[elasticmachine]

Pinging @elastic/es-core-infra

[talevy]

I am not familiar with the semantics of versionAdded. I haven't seen any issues
come up with TooManyBucketsException, so I am not sure what guarantees are
needed to be verified in tests for this change.

[DaveCTurner]

It's used here, to decide whether to serialise the exception "properly" or whether to use a NotSerializableExceptionWrapper based on the remote node's version:

elasticsearch/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java

Lines 948 to 952 in 787acb1

	if (throwable instanceof ElasticsearchException && ElasticsearchException.isRegistered(throwable.getClass(), version)) {
	ex = (ElasticsearchException) throwable;
	} else {
	ex = new NotSerializableExceptionWrapper(throwable);
	}

This means that, with today's master, a 7.0 node that wants to throw a SnapshotInProgressException at a 6.7 node will fall back to a NotSerializableExceptionWrapper, so that this line (in 6.x) doesn't trigger in a mixed cluster:

elasticsearch/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/AsyncRetryDuringSnapshotActionStep.java

Line 73 in c8905da

if (e instanceof SnapshotInProgressException) {

Unfortunately you can only really catch this with a mixed-cluster REST test.

I think we should consider adding an assertion that checks that once we've deserialised an exception from a remote node we believe that we could serialise it back to that node based on its version. I think that'd help catch this kind of asymmetry.

[talevy]

Cool, thanks. will look into adding this

[talevy]

@DaveCTurner I decided just to add unit tests for the serialization.

elasticsearch/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/AsyncRetryDuringSnapshotActionStep.java

Line 73 in c8905da

if (e instanceof SnapshotInProgressException) {

Not sure if the above line is really testable since all the actions in ILM are done on the same node. So either the master node requested to delete or close an index on an upgraded 6.7 node, in which case, it would pick this up with no issue.

This does make me realize that running these ILM steps against 6.6 nodes would cause things to fail. That being said, we are in the process of writing documentation basically saying that mixed-clusters is not supported

let me know what you think!

[talevy]

run elasticsearch-ci/default-distro

[talevy]

run elasticsearch-ci/1

[talevy]

hey @DaveCTurner, mind taking another look?

[DaveCTurner]

Sure, LGTM, sorry for the delay.

[talevy]

thanks David!