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

Skip coordinator in getWorkerMemoryInfo if not included in task processing #23079

Conversation

losipiuk
Copy link
Member

No description provided.

@losipiuk losipiuk added the no-release-notes This pull request does not require release notes entry label Aug 20, 2024
@cla-bot cla-bot bot added the cla-signed label Aug 20, 2024
@losipiuk losipiuk merged commit 5583b5f into trinodb:master Aug 20, 2024
94 of 95 checks passed
@github-actions github-actions bot added this to the 455 milestone Aug 20, 2024
@losipiuk
Copy link
Member Author

CI: #23080

losipiuk added a commit to losipiuk/trino that referenced this pull request Aug 26, 2024
The change to filter out coordinator from output of
ClusterMemoryManager::getWorkerMemoryInfo
(trinodb#23079) was beneficial for how
ExponentialGrowthPartitionMemoryEstimator behaves if coordinator is
differently sized than workers.

Yet it introduced regression as same method was used in
BinPackingNodeAllocatorService and as a result COORDINATOR_ONLY tasks
could not be scheduled at all if node-scheduler.include-coordinator was
set to false.

Unfortunately we had node-scheduler.include-coordinator set to true in
all the tests so it was not catch by automation.

This commit adds a regression test too.
losipiuk added a commit that referenced this pull request Aug 26, 2024
The change to filter out coordinator from output of
ClusterMemoryManager::getWorkerMemoryInfo
(#23079) was beneficial for how
ExponentialGrowthPartitionMemoryEstimator behaves if coordinator is
differently sized than workers.

Yet it introduced regression as same method was used in
BinPackingNodeAllocatorService and as a result COORDINATOR_ONLY tasks
could not be scheduled at all if node-scheduler.include-coordinator was
set to false.

Unfortunately we had node-scheduler.include-coordinator set to true in
all the tests so it was not catch by automation.

This commit adds a regression test too.
Joelg96 pushed a commit to Joelg96/trino that referenced this pull request Aug 26, 2024
The change to filter out coordinator from output of
ClusterMemoryManager::getWorkerMemoryInfo
(trinodb#23079) was beneficial for how
ExponentialGrowthPartitionMemoryEstimator behaves if coordinator is
differently sized than workers.

Yet it introduced regression as same method was used in
BinPackingNodeAllocatorService and as a result COORDINATOR_ONLY tasks
could not be scheduled at all if node-scheduler.include-coordinator was
set to false.

Unfortunately we had node-scheduler.include-coordinator set to true in
all the tests so it was not catch by automation.

This commit adds a regression test too.
mneethiraj pushed a commit to mneethiraj/trino that referenced this pull request Sep 2, 2024
The change to filter out coordinator from output of
ClusterMemoryManager::getWorkerMemoryInfo
(trinodb#23079) was beneficial for how
ExponentialGrowthPartitionMemoryEstimator behaves if coordinator is
differently sized than workers.

Yet it introduced regression as same method was used in
BinPackingNodeAllocatorService and as a result COORDINATOR_ONLY tasks
could not be scheduled at all if node-scheduler.include-coordinator was
set to false.

Unfortunately we had node-scheduler.include-coordinator set to true in
all the tests so it was not catch by automation.

This commit adds a regression test too.
jeschkies pushed a commit to jeschkies/trino that referenced this pull request Sep 10, 2024
The change to filter out coordinator from output of
ClusterMemoryManager::getWorkerMemoryInfo
(trinodb#23079) was beneficial for how
ExponentialGrowthPartitionMemoryEstimator behaves if coordinator is
differently sized than workers.

Yet it introduced regression as same method was used in
BinPackingNodeAllocatorService and as a result COORDINATOR_ONLY tasks
could not be scheduled at all if node-scheduler.include-coordinator was
set to false.

Unfortunately we had node-scheduler.include-coordinator set to true in
all the tests so it was not catch by automation.

This commit adds a regression test too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed no-release-notes This pull request does not require release notes entry
Development

Successfully merging this pull request may close these issues.

2 participants