Skip to content
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

Make StartReindexJobAction a non-master action #45217

Merged
merged 2 commits into from
Aug 6, 2019

Conversation

Tim-Brooks
Copy link
Contributor

@Tim-Brooks Tim-Brooks commented Aug 5, 2019

TransportStartReindexJobAction is currently a master action. Reindexing
only needs access to the cluster state to perform some validations.
Prior to persistent reindexing we used a normal data node to perform
these validations. There is no reason that these validations need a
perfectly update-to-date view of the cluster state. The commit changes
the action to be a normal transport action.

Relates to #42612.

TransportStartReindexJobAction is currently a master action. Reindexing
only needs access to the cluster state to perform some validations.
Prior to persistent reindexing we used a normal data node to perform
these validations. There is not reason that these validations need a
perfectly update to date view of the cluster state. The commit changes
the action to be a normal transport action.

Relates to elastic#42612.
@Tim-Brooks Tim-Brooks added >non-issue v8.0.0 :Distributed/Reindex Issues relating to reindex that are not caused by issues further down labels Aug 5, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Override
protected void masterOperation(Task task, StartReindexJobAction.Request request, ClusterState state,
ActionListener<StartReindexJobAction.Response> listener) {
protected void doExecute(Task task, StartReindexJobAction.Request request, ActionListener<StartReindexJobAction.Response> listener) {
// TODO: If the connection is lost to the master, this action might be retried creating two tasks.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this todo still accurate? I think this works OK now, since the REST action will call this on local node and thus there can be no retries?

@Tim-Brooks Tim-Brooks merged commit 96528f6 into elastic:reindex_v2 Aug 6, 2019
@Tim-Brooks Tim-Brooks deleted the not_master_action branch December 18, 2019 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Reindex Issues relating to reindex that are not caused by issues further down >non-issue v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants