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

Fix duplicate TE allocation to multiple workers with different resource spec matching the same SKU #599

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

fdc-ntflx
Copy link
Collaborator

Context

This PR addresses a problem encountered with TE allocation in scenarios where jobs have TEs with different resource specs that match the same SKU. For instance, if a job contains a TE with 1 core and another with 2 cores, and the smallest SKU available is a 2 core, both TEs will match the smallest SKU. This situation can lead to the same TE being incorrectly allocated to different workers, as currently the logic assumes that resource spec always matches the SKU size (which in a few cases it's not the case).

In order to address this issue, we now pass the list of already assigned TEs over to the matching logic, ensuring we aren't reusing them for other worker requests.

Checklist

  • ./gradlew build compiles code correctly
  • Added new tests where applicable
  • ./gradlew test passes all tests
  • Extended README or added javadocs where applicable

Copy link

github-actions bot commented Dec 11, 2023

Test Results

488 tests  +1   482 ✔️ +1   7m 32s ⏱️ +16s
131 suites ±0       6 💤 ±0 
131 files   ±0       0 ±0 

Results for commit 3c91d96. ± Comparison against base commit 6025b7b.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@sundargates sundargates left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link

github-actions bot commented Dec 11, 2023

Uploaded Artifacts

To use these artifacts in your Gradle project, paste the following lines in your build.gradle.

resolutionStrategy {
    force "io.mantisrx:mantis-common:0.1.0-20231212.091549-460"
    force "io.mantisrx:mantis-discovery-proto:0.1.0-20231212.091549-460"
    force "io.mantisrx:mantis-network:0.1.0-20231212.091549-460"
    force "io.mantisrx:mantis-remote-observable:0.1.0-20231212.091549-461"
    force "io.mantisrx:mantis-runtime:0.1.0-20231212.091549-461"
    force "io.mantisrx:mantis-common-serde:0.1.0-20231212.091549-460"
    force "io.mantisrx:mantis-client:0.1.0-20231212.091549-461"
    force "io.mantisrx:mantis-runtime-loader:0.1.0-20231212.091549-461"
    force "io.mantisrx:mantis-shaded:0.1.0-20231212.091549-459"
    force "io.mantisrx:mantis-testcontainers:0.1.0-20231212.091549-130"
    force "io.mantisrx:mantis-connector-iceberg:0.1.0-20231212.091549-459"
    force "io.mantisrx:mantis-connector-publish:0.1.0-20231212.091549-460"
    force "io.mantisrx:mantis-control-plane-client:0.1.0-20231212.091549-460"
    force "io.mantisrx:mantis-connector-job:0.1.0-20231212.091549-461"
    force "io.mantisrx:mantis-control-plane-core:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-examples-core:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-examples-groupby-sample:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-control-plane-server:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-examples-jobconnector-sample:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-examples-mantis-publish-sample:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-examples-sine-function:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-connector-kafka:0.1.0-20231212.091549-461"
    force "io.mantisrx:mantis-examples-synthetic-sourcejob:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-examples-wordcount:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-publish-core:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-publish-netty:0.1.0-20231212.091549-453"
    force "io.mantisrx:mantis-examples-twitter-sample:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-publish-netty-guice:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-server-worker-client:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-server-agent:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-server-worker:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-source-job-publish:0.1.0-20231212.091549-454"
    force "io.mantisrx:mantis-source-job-kafka:0.1.0-20231212.091549-454"
}

@fdc-ntflx fdc-ntflx force-pushed the fix-use-same-te-for-similar-workers branch from 22471a7 to 9b58f6b Compare December 11, 2023 21:02
@fdc-ntflx fdc-ntflx temporarily deployed to Integrate Pull Request December 11, 2023 21:02 — with GitHub Actions Inactive
@fdc-ntflx fdc-ntflx force-pushed the fix-use-same-te-for-similar-workers branch from 9b58f6b to 3c91d96 Compare December 12, 2023 09:12
@fdc-ntflx fdc-ntflx temporarily deployed to Integrate Pull Request December 12, 2023 09:13 — with GitHub Actions Inactive
@fdc-ntflx fdc-ntflx merged commit d6aca28 into master Dec 12, 2023
7 checks passed
@fdc-ntflx fdc-ntflx deleted the fix-use-same-te-for-similar-workers branch December 12, 2023 09:25
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.

None yet

2 participants