Ensure to access RecoveryState#fileDetails under lock

[paulward24]

The field fileDetails (a HashMap, i.e., not thread safe)

https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/indices/recovery/RecoveryState.java#L679

is used only in synchronzied methods (in about 20 locations), e.g.,:

https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/indices/recovery/RecoveryState.java#L767-L768

i.e., including .size().

This is correct, because according to JDK:

If multiple threads access a hash map concurrently, and at least
one of the threads modifies the map structurally, it must be
synchronized externally.

https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html

However, in the 21st location, here:

https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/indices/recovery/RecoveryState.java#L958

the method is not synchronized.

This CR simply adds the keyword synchronized to the method, just like for all the other places.

[dnhatn]

@paulward24 Can you please sign CLA? Thank you for reporting and working on this issue.

[elasticmachine]

Pinging @elastic/es-distributed

[paulward24]

I signed the CLA.

Thanks for pointing this out!

[dnhatn]

LGTM

[dnhatn]

@elasticmachine test this please

[dnhatn]

@elasticmachine test this please

[original-brownbear]

Jenkins run elasticsearch-ci/1

[paulward24]

I see 2 tests failing.

I don't know why this would happen, but it seems unlikely that they fail from this patch.

I.e., the patch is just synchronizing the get() of a HashMap --- there is nothing to deadlock or break there.

I am not familiar with the internals of ElasticSearch to be able to debug those two tests

Nhat, can you please take a look?

Thanks!!

[original-brownbear]

I think the fix for the failures here is incoming in #43861

[original-brownbear]

@paulward24 could you merge master into your branch please so we can try building again? The test failure should be fixed in master now.

[dnhatn]

@elasticmachine update branch

[paulward24]

I see this "@elasticmachine update branch" from Nhat.

Should I still do the merge?

[dnhatn]

Yes. please merge master into your branch. Thank you!

[paulward24]

Ok, I did --- how do I update this CR ?

[dnhatn]

@elasticmachine test this please

[paulward24]

@elasticmachine test this please

[paulward24]

ok, I don't know why these tests fail.

Unlikely to be related to the patch

[original-brownbear]

Jenkins test this

(@paulward24 yea the failures are the result of a temporary infrastructure issue we were experiencing, let's see if it passed :))

[original-brownbear]

Jenkins run elasticsearch-ci/packaging-sample

[dnhatn]

@paulward24 Thanks again for working on this :).

[paulward24]

Thank you Nhat and Armin for all the hard work on this!