Remove isRecovering method from Engine

[dnhatn]

We already prevent flushing in Engine if it's recovering. Hence we can remove the protection in IndexShard.

[elasticmachine]

Pinging @elastic/es-distributed

[henningandersen]

We used to throw IllegalIndexShardStateException and now throw IllegalStateException. The former has a certain semantics, among others TransportActions.isShardNotAvailableException returns true for it. This seems to affect error handling in replication actions, including TransportFlushAction. Looks like we simply ignore recovering shards rather than fail on them (which makes sense now that we flush after recovery).

Also, looking at the original PR introducing this:
#20597 (comment)
both checks were added in the same commit and this was discussed at the time.

On the other hand no tests fail, which could indicate the change is OK (or we lack tests). And if there really is no difference, the check should be removed. I am only a bit hesitant given above and seek additional input on this.

[dnhatn]

@henningandersen Thank you for reviewing.

I also looked at that PR. We introduced this method as a workaround (#20597 (comment)) for IndexingMemoryController, but it is not required now.

Moreover, with #28049 we stop sending replication requests to recovering shards.

Perhaps it's safer to make this change in 8.0 only.

[henningandersen]

Thanks @dnhatn for elaborating, I can no longer see any reason why we cannot remove this.

So LGTM, but I think we need an additional reviewer (@ywelsch ) on this.

[ywelsch]

LGTM. Thanks for providing the necessary context, made it much easier to review.

It's ok for this to go into 7.5

[dnhatn]

Thanks @henningandersen + @ywelsch.