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 scheduler double booking task executors #733

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Conversation

Andyz26
Copy link
Collaborator

@Andyz26 Andyz26 commented Dec 17, 2024

Context

Problem:
When a TE is returned from here to be used for scheduling, its state remain active until the scheduler trigger another message to update (lock) the state. However when large number of the requests are active at the same time on same sku, the gap between here and the message to lock the state can be large so another schedule request message can be in between and got the same set of TEs.

Solution:
To avoid this, a lease is added to each TE state (based on last scheduled usage timestamp) to temporarily lock the TE to be used again. Since this is only lock between actor messages and lease duration can be short.

  • Also fixed a NPE from the node state update call.

Checklist

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

@Andyz26 Andyz26 had a problem deploying to Integrate Pull Request December 17, 2024 00:24 — with GitHub Actions Failure
@Andyz26 Andyz26 changed the title Fix scheduler double booking task executoers Fix scheduler double booking task executors Dec 17, 2024
Copy link

github-actions bot commented Dec 17, 2024

Test Results

618 tests  ±0   608 ✅ ±0   7m 42s ⏱️ +3s
142 suites ±0    10 💤 ±0 
142 files   ±0     0 ❌ ±0 

Results for commit b635b8f. ± Comparison against base commit 735435f.

♻️ This comment has been updated with latest results.

@@ -278,6 +283,15 @@ Instant getLastActivity() {
return this.lastActivity;
}

Duration getLastSchedulerLeasedDuration()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Format

Suggested change
Duration getLastSchedulerLeasedDuration()
Duration getLastSchedulerLeasedDuration() {

@Andyz26 Andyz26 had a problem deploying to Integrate Pull Request December 17, 2024 23:11 — with GitHub Actions Failure
@Andyz26 Andyz26 merged commit e90032d into master Dec 17, 2024
4 of 5 checks passed
@Andyz26 Andyz26 deleted the andyz/fixNodeDisable1216 branch December 17, 2024 23: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.

2 participants