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

Block reconciliation if queue nearly full #2185

Merged
merged 5 commits into from
Mar 12, 2021

Conversation

rosalind210
Copy link
Contributor

If the queue is nearly full, adding a number of tasks for reconciliation can put us into a loop of lag as the scheduler tries to go through tasks and more reconciliation attempts.

@ssalinas
Copy link
Member

ssalinas commented Mar 5, 2021

let's test in staging, but looks good 👍 . We can do something like shrink the queue size really small + make an example request and scale it to something ridiculous like 300 to hopefully trigger and see it in action

@rosalind210 rosalind210 added the staging Merged to staging branch label Mar 5, 2021
@@ -647,4 +647,12 @@ public boolean hasRoomForMoreUpdates() {
statusUpdatesExecutor.getExecutorService()
);
}

public double getQueueFullness() {
LOG.info("Queue size: {}", statusUpdatesExecutor.getQueue().size());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should I add more, like queue limit and the computation of queue fullness?

@rosalind210
Copy link
Contributor Author

I tried adding unit tests, but the status queue was still processing them faster than I could run a reconciliation check so I wasn't able to hit the blocking conditional; and if I tried to create too many tasks, I was blocked by testing resources.

@ssalinas
Copy link
Member

🚢

@ssalinas
Copy link
Member

🚢

@rosalind210 rosalind210 merged commit 1cbf9df into master Mar 12, 2021
@ssalinas ssalinas deleted the stop_reconcile_when_queue_near_full branch March 12, 2021 14:20
@ssalinas ssalinas added this to the 1.5.0 milestone May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
staging Merged to staging branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants