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

Clean up & streamline RestIndicesAction #93437

Conversation

DaveCTurner
Copy link
Contributor

  • No need to wait for the get-settings call to return before sending the other requests.

  • No need to request cluster health at all, we can compute this locally from the cluster state.

  • No need to get the entire cluster state, we only need the metadata and routing table.

  • No need to put all the responses into an untyped list and then cast them out again.

- No need to wait for the get-settings call to return before sending the
  other requests.

- No need to request cluster health at all, we can compute this locally
  from the cluster state.

- No need to get the entire cluster state, we only need the metadata and
  routing table.

- No need to put all the responses into an untyped list and then cast
  them out again.
@DaveCTurner DaveCTurner added >non-issue :Data Management/Indices APIs APIs to create and manage indices and templates v8.7.0 labels Feb 2, 2023
@DaveCTurner DaveCTurner requested a review from tlrx February 2, 2023 07:52
@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Feb 2, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing my messy code 😞

@@ -99,7 +99,7 @@ public RefCountingListener(int maxExceptions, ActionListener<Void> delegate) {
assert false : maxExceptions;
throw new IllegalArgumentException("maxExceptions must be positive");
}
this.delegate = Objects.requireNonNull(delegate);
this.delegate = ActionListener.assertOnce(Objects.requireNonNull(delegate));
Copy link
Member

Choose a reason for hiding this comment

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

👍

@DaveCTurner
Copy link
Contributor Author

Guess who didn't even compile the tests before opening this? 🤡

@DaveCTurner DaveCTurner added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Feb 7, 2023
@elasticsearchmachine elasticsearchmachine merged commit 464251c into elastic:main Feb 7, 2023
@DaveCTurner DaveCTurner deleted the 2023-02-02-RestIndicesAction-grouped-listener branch February 7, 2023 14:17
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Sep 22, 2023
Similar to elastic#93437 we can run all the sub-requests in parallel.
DaveCTurner added a commit that referenced this pull request Oct 6, 2023
Similar to #93437 we can run all the sub-requests in parallel.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Data Management/Indices APIs APIs to create and manage indices and templates >non-issue Team:Data Management Meta label for data/management team v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants