Make cluster state external to o.e.c.a.s.ShardStateAction #15736
Conversation
|
The primary motivator for this change is ongoing work for the next step of #14252 (handling when we send a shard failure to a node that we thought was the master but is no longer the master). |
This commit modifies the handling of cluster states in o.e.c.a.s.ShardStateAction so that all necessary state is obtained externally to the ShardStateAction#shardFailed and ShardStateAction#shardStarted methods. This refactoring permits the removal of the ClusterService field from ShardStateAction.
|
Makes sense to me but I suspect @bleskes will give final review. |
| } | ||
|
|
||
| private void innerShardFailed(final ShardRouting shardRouting, final String indexUUID, final DiscoveryNode masterNode, final String message, final Throwable failure, TimeValue timeout, Listener listener) { | ||
| private void innerShardFailed(final ClusterState clusterState, final ShardRouting shardRouting, final String indexUUID, final String message, final Throwable failure, TimeValue timeout, Listener listener) { |
There was a problem hiding this comment.
does this inner shard failed buy us anything now? can't it be folded into one of the shardFailed variants?
There was a problem hiding this comment.
Yes, I was leaning towards this last night as well and I'm glad to see that you think the same.
|
Left some very minor comments. All in the spirit of clean ups - which is what this Pr seems to be about... |
This commit restores logging the ShardRouting#shardId at the front of the log messages in ShardStateAction. The reason for this is so that shard-level log messages have the format "[component][node][shard] message".
|
@bleskes Thanks for the comments and additional suggestions. I've pushed more commits to address. |
| @@ -158,7 +164,17 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS | |||
| } | |||
| } | |||
|
|
|||
| class ShardFailedClusterStateHandler implements ClusterStateTaskExecutor<ShardRoutingEntry> { | |||
| private static class ShardFailedClusterStateHandler implements ClusterStateTaskExecutor<ShardRoutingEntry> { | |||
There was a problem hiding this comment.
nit: I know this was already the name, but can we call this ShardFailedClusterStateTaskExecutor? It will help also with it being a member of ShardFailedTransportHandler .
|
LGTM. Left one minor naming suggestion, no need for another review. |
…state-refactoring Make cluster state external to o.e.c.a.s.ShardStateAction
This commit modifies the handling of cluster states in
o.e.c.a.s.ShardStateAction so that all necessary state is obtained
externally to the ShardStateAction#shardFailed and
ShardStateAction#shardStarted methods. This refactoring permits the
removal of the ClusterService field from ShardStateAction.