Skip to content

fix: consolidator waiter cap fallback to independent execution#716

Merged
tanjinx merged 7 commits intoslack-19.0from
fix-consolidator-waiter-cap
Oct 24, 2025
Merged

fix: consolidator waiter cap fallback to independent execution#716
tanjinx merged 7 commits intoslack-19.0from
fix-consolidator-waiter-cap

Conversation

@tanjinx
Copy link
Copy Markdown

@tanjinx tanjinx commented Oct 21, 2025

Fix bug introduced by vitessio#17244

When the consolidator waiter cap is reached, queries should fall back to independent execution instead of returning empty results.

Before this fix:

  • Queries exceeding waiter cap would skip waiting for consolidation
  • They would immediately try to access q.Result() before completion
  • This resulted in empty/incomplete results being returned

After this fix:

  • Queries exceeding waiter cap fall back to regular execution path
  • All queries return correct results regardless of consolidation status
  • Waiter cap configuration still controls resource usage as intended

Changes:

  • Modified execSelect() in query_executor.go to implement fallback logic
  • Enhanced FakePendingResult to properly simulate waiter count behavior
  • Added comprehensive test TestQueryExecutorConsolidatorWaiterCapFallback

Description

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported to release branches
  • If this change is to be back-ported to previous releases, a justification is included in the PR description
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on CI?
  • Documentation was added or is not required

Deployment Notes

AI Disclosure

@github-actions github-actions bot added this to the v19.0.7 milestone Oct 21, 2025
When the consolidator waiter cap is reached, queries should fall back
to independent execution instead of returning empty results.

Before this fix:
- Queries exceeding waiter cap would skip waiting for consolidation
- They would immediately try to access q.Result() before completion
- This resulted in empty/incomplete results being returned

After this fix:
- Queries exceeding waiter cap fall back to regular execution path
- All queries return correct results regardless of consolidation status
- Waiter cap configuration still controls resource usage as intended

Changes:
- Modified execSelect() in query_executor.go to implement fallback logic
- Enhanced FakePendingResult to properly simulate waiter count behavior
- Added comprehensive test TestQueryExecutorConsolidatorWaiterCapFallback

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
@tanjinx tanjinx force-pushed the fix-consolidator-waiter-cap branch from 28bcbd7 to 50eebde Compare October 21, 2025 00:34
@tanjinx tanjinx marked this pull request as ready for review October 21, 2025 00:35
@tanjinx tanjinx requested a review from a team as a code owner October 21, 2025 00:35
@tanjinx tanjinx requested a review from rvrangel October 21, 2025 00:40
tanjinx and others added 5 commits October 21, 2025 14:07
Signed-off-by: Tanjin Xu <tanjin.xu@slack-corp.com>
Signed-off-by: Tanjin Xu <tanjin.xu@slack-corp.com>
Add --consolidator-query-waiter-cap-method flag to control behavior when
consolidator waiter cap is exceeded. Options:
- 'fallthrough' (default): Fall back to independent query execution
- 'reject': Return RESOURCE_EXHAUSTED error

This provides operators fine-grained control over consolidator memory
management while maintaining backward compatibility.

Changes:
- Add ConsolidatorQueryWaiterCapMethod config field and CLI flag
- Update execSelect() to handle both reject and fallthrough behaviors
- Add comprehensive test coverage for both methods
- Add config validation with graceful defaults
- Fix waiter counter cleanup to ensure proper resource management

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
@tanjinx tanjinx merged commit 29faa27 into slack-19.0 Oct 24, 2025
163 of 168 checks passed
@tanjinx tanjinx deleted the fix-consolidator-waiter-cap branch October 24, 2025 17:05
tanjinx added a commit that referenced this pull request Oct 24, 2025
* fix: consolidator waiter cap fallback to independent execution

When the consolidator waiter cap is reached, queries should fall back
to independent execution instead of returning empty results.

Before this fix:
- Queries exceeding waiter cap would skip waiting for consolidation
- They would immediately try to access q.Result() before completion
- This resulted in empty/incomplete results being returned

After this fix:
- Queries exceeding waiter cap fall back to regular execution path
- All queries return correct results regardless of consolidation status
- Waiter cap configuration still controls resource usage as intended

Changes:
- Modified execSelect() in query_executor.go to implement fallback logic
- Enhanced FakePendingResult to properly simulate waiter count behavior
- Added comprehensive test TestQueryExecutorConsolidatorWaiterCapFallback

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

* implement consolidator-query-waiter-cap-method

* fix help message

Signed-off-by: Tanjin Xu <tanjin.xu@slack-corp.com>

* fix redundant code

* fix test

Signed-off-by: Tanjin Xu <tanjin.xu@slack-corp.com>

* feat: add consolidator-query-waiter-cap-method config parameter

Add --consolidator-query-waiter-cap-method flag to control behavior when
consolidator waiter cap is exceeded. Options:
- 'fallthrough' (default): Fall back to independent query execution
- 'reject': Return RESOURCE_EXHAUSTED error

This provides operators fine-grained control over consolidator memory
management while maintaining backward compatibility.

Changes:
- Add ConsolidatorQueryWaiterCapMethod config field and CLI flag
- Update execSelect() to handle both reject and fallthrough behaviors
- Add comprehensive test coverage for both methods
- Add config validation with graceful defaults
- Fix waiter counter cleanup to ensure proper resource management

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

---------

Signed-off-by: Tanjin Xu <tanjin.xu@slack-corp.com>
Co-authored-by: Claude <svc-devxp-claude@slack-corp.com>
tanjinx added a commit that referenced this pull request Oct 31, 2025
…#726)

* fix: consolidator waiter cap fallback to independent execution

When the consolidator waiter cap is reached, queries should fall back
to independent execution instead of returning empty results.

Before this fix:
- Queries exceeding waiter cap would skip waiting for consolidation
- They would immediately try to access q.Result() before completion
- This resulted in empty/incomplete results being returned

After this fix:
- Queries exceeding waiter cap fall back to regular execution path
- All queries return correct results regardless of consolidation status
- Waiter cap configuration still controls resource usage as intended

Changes:
- Modified execSelect() in query_executor.go to implement fallback logic
- Enhanced FakePendingResult to properly simulate waiter count behavior
- Added comprehensive test TestQueryExecutorConsolidatorWaiterCapFallback



* implement consolidator-query-waiter-cap-method

* fix help message



* fix redundant code

* fix test



* feat: add consolidator-query-waiter-cap-method config parameter

Add --consolidator-query-waiter-cap-method flag to control behavior when
consolidator waiter cap is exceeded. Options:
- 'fallthrough' (default): Fall back to independent query execution
- 'reject': Return RESOURCE_EXHAUSTED error

This provides operators fine-grained control over consolidator memory
management while maintaining backward compatibility.

Changes:
- Add ConsolidatorQueryWaiterCapMethod config field and CLI flag
- Update execSelect() to handle both reject and fallthrough behaviors
- Add comprehensive test coverage for both methods
- Add config validation with graceful defaults
- Fix waiter counter cleanup to ensure proper resource management



---------

Signed-off-by: Tanjin Xu <tanjin.xu@slack-corp.com>
Co-authored-by: Claude <svc-devxp-claude@slack-corp.com>
tanjinx added a commit that referenced this pull request Nov 10, 2025
…#726)

* fix: consolidator waiter cap fallback to independent execution

When the consolidator waiter cap is reached, queries should fall back
to independent execution instead of returning empty results.

Before this fix:
- Queries exceeding waiter cap would skip waiting for consolidation
- They would immediately try to access q.Result() before completion
- This resulted in empty/incomplete results being returned

After this fix:
- Queries exceeding waiter cap fall back to regular execution path
- All queries return correct results regardless of consolidation status
- Waiter cap configuration still controls resource usage as intended

Changes:
- Modified execSelect() in query_executor.go to implement fallback logic
- Enhanced FakePendingResult to properly simulate waiter count behavior
- Added comprehensive test TestQueryExecutorConsolidatorWaiterCapFallback

* implement consolidator-query-waiter-cap-method

* fix help message

* fix redundant code

* fix test

* feat: add consolidator-query-waiter-cap-method config parameter

Add --consolidator-query-waiter-cap-method flag to control behavior when
consolidator waiter cap is exceeded. Options:
- 'fallthrough' (default): Fall back to independent query execution
- 'reject': Return RESOURCE_EXHAUSTED error

This provides operators fine-grained control over consolidator memory
management while maintaining backward compatibility.

Changes:
- Add ConsolidatorQueryWaiterCapMethod config field and CLI flag
- Update execSelect() to handle both reject and fallthrough behaviors
- Add comprehensive test coverage for both methods
- Add config validation with graceful defaults
- Fix waiter counter cleanup to ensure proper resource management

---------

Signed-off-by: Tanjin Xu <tanjin.xu@slack-corp.com>
Co-authored-by: Claude <svc-devxp-claude@slack-corp.com>
sbaker617 pushed a commit that referenced this pull request Feb 5, 2026
…#726)

* fix: consolidator waiter cap fallback to independent execution

When the consolidator waiter cap is reached, queries should fall back
to independent execution instead of returning empty results.

Before this fix:
- Queries exceeding waiter cap would skip waiting for consolidation
- They would immediately try to access q.Result() before completion
- This resulted in empty/incomplete results being returned

After this fix:
- Queries exceeding waiter cap fall back to regular execution path
- All queries return correct results regardless of consolidation status
- Waiter cap configuration still controls resource usage as intended

Changes:
- Modified execSelect() in query_executor.go to implement fallback logic
- Enhanced FakePendingResult to properly simulate waiter count behavior
- Added comprehensive test TestQueryExecutorConsolidatorWaiterCapFallback

* implement consolidator-query-waiter-cap-method

* fix help message

* fix redundant code

* fix test

* feat: add consolidator-query-waiter-cap-method config parameter

Add --consolidator-query-waiter-cap-method flag to control behavior when
consolidator waiter cap is exceeded. Options:
- 'fallthrough' (default): Fall back to independent query execution
- 'reject': Return RESOURCE_EXHAUSTED error

This provides operators fine-grained control over consolidator memory
management while maintaining backward compatibility.

Changes:
- Add ConsolidatorQueryWaiterCapMethod config field and CLI flag
- Update execSelect() to handle both reject and fallthrough behaviors
- Add comprehensive test coverage for both methods
- Add config validation with graceful defaults
- Fix waiter counter cleanup to ensure proper resource management

---------

Signed-off-by: Tanjin Xu <tanjin.xu@slack-corp.com>
Co-authored-by: Claude <svc-devxp-claude@slack-corp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants