-
Notifications
You must be signed in to change notification settings - Fork 24.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Close Index API] Add TransportShardCloseAction for pre-closing verifications #36249
[Close Index API] Add TransportShardCloseAction for pre-closing verifications #36249
Conversation
Pinging @elastic/es-distributed |
import org.elasticsearch.threadpool.ThreadPool; | ||
import org.elasticsearch.transport.TransportService; | ||
|
||
public class TransportShardCloseAction extends TransportReplicationAction<ShardCloseRequest, ShardCloseRequest, ReplicationResponse> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hesitated on the name of the action and request as it does not really close the shard. Suggestions welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TransportVerifyShardBeforeCloseAction
? I know it's long, but it's OK, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion - verbose but better than TransportShardCloseAction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Left some nits.
import org.elasticsearch.threadpool.ThreadPool; | ||
import org.elasticsearch.transport.TransportService; | ||
|
||
public class TransportShardCloseAction extends TransportReplicationAction<ShardCloseRequest, ShardCloseRequest, ReplicationResponse> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TransportVerifyShardBeforeCloseAction
? I know it's long, but it's OK, I think.
import org.elasticsearch.action.support.replication.ReplicationRequest; | ||
import org.elasticsearch.index.shard.ShardId; | ||
|
||
public class ShardCloseRequest extends ReplicationRequest<ShardCloseRequest> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just inline this in the transport action file? I don't see the need for another file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
final IndicesService indicesService, final ThreadPool threadPool, final ShardStateAction stateAction, | ||
final ActionFilters actionFilters, final IndexNameExpressionResolver indexNameExpressionResolver) { | ||
super(settings, NAME, transportService, clusterService, indicesService, threadPool, stateAction, | ||
actionFilters, indexNameExpressionResolver, ShardCloseRequest::new, ShardCloseRequest::new, ThreadPool.Names.GENERIC); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want a management thread pool here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
+ "] mismatches maximum sequence number [" + maxSeqNo + "] on index shard " + shardId); | ||
} | ||
indexShard.flush(new FlushRequest()); | ||
logger.debug("shard {} is ready for closing", shardId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we put the shardId first in the log line so it will look like [index][0] shard is ready for closing
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
c5a0dff
to
d3e786d
Compare
d3e786d
to
feac528
Compare
Thanks @bleskes |
The commit changes how indices are closed in the MetaDataIndexStateService. It now uses a 3 steps process where writes are blocked on indices to be closed, then some verifications are done on shards using the TransportVerifyShardBeforeCloseAction added in #36249, and finally indices states are moved to CLOSE and their routing tables removed. The closing process also takes care of using the pre-7.0 way to close indices if the cluster contains mixed version of nodes and a node does not support the TransportVerifyShardBeforeCloseAction. It also closes unassigned indices. Related to #33888
This commit backports to 6.x of the Close Index API refactoring. It cherry-picks the following commits from master: 3ca885e [Close Index API] Add TransportShardCloseAction for pre-closing verifications (#36249) 8e5dd20 [Close Index API] Refactor MetaDataIndexStateService (#36354) 7372529 [Tests] Reduce randomization in CloseWhileRelocatingShardsIT (#36694) 103c4d4 [Close Index API] Mark unavailable shard copy as stale during verification (#36755) 1959388 [Close Index API] Propagate tasks ids between Freeze, Close and Verify(#36630) e149b08 [Close Index API] Add unique UUID to ClusterBlock (#36775) dc371ef [Tests] Fix ReopenWhileClosingIT with correct min num shards The following two commits were needed to adapt the change to 6.x: ef6ae69 [Close Index API] Adapt MetaDataIndexStateServiceTests after merge 21b7653 [Tests] Adapt CloseIndexIT tests for 6.x Related to #33888
Note: this pull request will be merged in the
close-index-api-refactoring
branchThis pull request adds the
TransportShardCloseAction
which is a transport replication action that acquires all index shard permits for its execution. This action will be used in the future by theMetaDataIndexStateService
in a new index closing process, where we need to execute some sanity checks before closing an index.The action executes the following verifications on the primary and replicas:
When the verifications are done and successful, the shard is flushed.
Once this pull request is merged, a follow up pull request will change the
MetaDataIndexStateService
to make use of this new transport action.