Skip to content

Conversation

@jungeunyooon
Copy link

Description

This PR migrates the gRPC transport executor from FixedExecutorBuilder to ForkJoinPoolExecutorBuilder to improve performance through work-stealing and better load balancing.

  • Updated GrpcPlugin.java to use ForkJoinPoolExecutorBuilder instead of FixedExecutorBuilder
  • Updated test files to reflect the executor change

Related Issues

Resolves #19370

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@jungeunyooon jungeunyooon requested a review from a team as a code owner October 20, 2025 05:19
@github-actions github-actions bot added enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers Search:Performance labels Oct 20, 2025
@jungeunyooon jungeunyooon force-pushed the feature/19370-migrate-grpc-to-forkjoinpool branch from 42def60 to cb0269f Compare October 20, 2025 05:26
@github-actions
Copy link
Contributor

❌ Gradle check result for cb0269f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@jungeunyooon jungeunyooon force-pushed the feature/19370-migrate-grpc-to-forkjoinpool branch from cb0269f to 32618f4 Compare October 20, 2025 05:41
@github-actions
Copy link
Contributor

❌ Gradle check result for 32618f4: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

@karenyrx karenyrx left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on!

- Onboarding new maven snapshots publishing to s3 ([#19619](https://github.com/opensearch-project/OpenSearch/pull/19619))
- Remove MultiCollectorWrapper and use MultiCollector in Lucene instead ([#19595](https://github.com/opensearch-project/OpenSearch/pull/19595))
- Change implementation for `percentiles` aggregation for latency improvement ([#19648](https://github.com/opensearch-project/OpenSearch/pull/19648))
- Migrate gRPC transport executor from FixedExecutorBuilder to ForkJoinPoolExecutorBuilder for improved performance ([#19370](https://github.com/opensearch-project/OpenSearch/issues/19370))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Migrate gRPC transport executor from FixedExecutorBuilder to ForkJoinPoolExecutorBuilder for improved performance ([#19370](https://github.com/opensearch-project/OpenSearch/issues/19370))
- Migrate gRPC transport executor from FixedExecutorBuilder to ForkJoinPoolExecutorBuilder for improved performance ([#19685](https://github.com/opensearch-project/OpenSearch/issues/19685))

*
* @param settings The node settings
* @param threadPool The thread pool
* @param settings The node settings
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you run ./gradlew spotlessApply to remove all the formatting changes in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

./gradlew precommit can also be run locally to ensure the Gradle Check CI passes on the PR before pushing

// Create a ThreadPool with the gRPC executor
Settings settings = Settings.builder().put("node.name", "test-node").put("grpc.netty.executor_count", 4).build();
ExecutorBuilder<?> grpcExecutorBuilder = new FixedExecutorBuilder(settings, "grpc", 4, 1000, "thread_pool.grpc");
ExecutorBuilder<?> grpcExecutorBuilder = new ForkJoinPoolExecutorBuilder("grpc", 4, "thread_pool.grpc");
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you run start a local cluster, then run this API and share the output to ensure this change is working as expected?
https://github.com/opensearch-project/OpenSearch/blob/main/modules/transport-grpc/README.md#thread-pool-monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers Search:Performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Migrate gRPC transport executor to ForkJoinPool for improved performance

2 participants