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

participant-integration-api: Reuse the services execution context for data munging. #8415

Conversation

ghost
Copy link

@ghost ghost commented Jan 6, 2021

This is the obvious follow-up to #7945. We can share the work-stealing execution pool between the indexer and the API server when they run in the same process.

Pull Request Checklist

  • Read and understand the contribution guidelines
  • Include appropriate tests
  • Set a descriptive title and thorough description
  • Add a reference to the issue this PR will solve, if appropriate
  • Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
  • Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

@cocreature
Copy link
Contributor

/azp run

@azure-pipelines
Copy link
Contributor

Pull request contains merge conflicts.

@ghost ghost force-pushed the samir/ledger/participant-integration-api/shared-services-execution-context branch from 6b0a7ef to 09c0408 Compare January 20, 2021 10:16
@ghost
Copy link
Author

ghost commented Jan 20, 2021

In testing on proprietary code, this seems to get about a 5% performance improvement, which I'm putting down to less CPU contention.

@ghost ghost marked this pull request as ready for review January 20, 2021 12:59
@ghost ghost self-requested a review January 20, 2021 12:59
@matthiasS-da matthiasS-da self-assigned this Jan 20, 2021
@matthiasS-da matthiasS-da self-requested a review January 21, 2021 12:47
Copy link
Contributor

@matthiasS-da matthiasS-da left a comment

Choose a reason for hiding this comment

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

Makes sense. Thank you!

Rather than spawning a separate thread pool for manipulating data before
and after index queries, we can just re-use the services EC, which is
already a work-stealing thread pool.
@mziolekda mziolekda force-pushed the samir/ledger/participant-integration-api/shared-services-execution-context branch from 09c0408 to cddfea7 Compare January 28, 2021 11:08
Copy link
Contributor

@mziolekda mziolekda left a comment

Choose a reason for hiding this comment

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

Looks good

@ghost ghost added the automerge label Jan 29, 2021
@mergify mergify bot merged commit a325d00 into main Jan 29, 2021
@mergify mergify bot deleted the samir/ledger/participant-integration-api/shared-services-execution-context branch January 29, 2021 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants