fix: resolve lite mode backend issues and add sequential strategy default#565
Merged
Conversation
Deploying gate-minekube with
|
| Latest commit: |
33febf6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://31a559dd.gate-minekube.pages.dev |
| Branch Preview URL: | https://fix-lite-backend-health-trac.gate-minekube.pages.dev |
This minimal fix addresses issue #564 by simply increasing the log verbosity level for failed backend connection attempts. This prevents console spam when backends are unreachable while maintaining the simple retry behavior that has always worked. Changes: - Failed backend connections now log at V(1) debug level instead of info - Fallback status messages also use V(1) to reduce spam - No complex health tracking or caching (keeps it simple) - Preserves existing retry behavior without marking backends unhealthy Fixes #564
a499514 to
759d9bb
Compare
This commit provides a complete fix for all three issues: 1. **Log spam reduction** (✓ Fixed) - Failed backend logs now use V(1) debug level - Fallback messages also use V(1) to reduce verbosity 2. **Backend cycling** (✓ Fixed) - When a backend fails, it's removed from the retry list - Strategy manager properly cycles through all available backends - No duplicate attempts on the same backend 3. **Fallback response** (✓ Fixed) - Fallback properly shown when all backends are unreachable - Already worked, just needed reduced log verbosity Changes: - Modified tryBackends logging to use V(1) for failed attempts - Fixed nextBackend to remove tried backends from the list - Added comprehensive tests for all three issues - Maintains simple retry behavior without complex health tracking Tests added: - TestBackendSelection_TriesAllBackends - TestBackendSelection_SucceedsOnSecondBackend - TestBackendSelection_NoDuplicateAttempts - TestFallbackResponse_UsedWhenAllBackendsFail - TestLogSpamReduction All existing tests continue to pass.
- Extracted handleFallbackResponse for better testability - Removed useless tests that didn't test real functionality: * TestResolveStatusResponseIntegration (just logged messages) * TestBackendRemovalFromList (tested list ops, not real code) - Added meaningful integration tests: * TestNextBackendFunctionality - tests actual nextBackend implementation * TestFallbackResponseWithRealRoute - tests real fallback scenarios * TestLogVerbosityActuallyWorks - verifies log.V(1) behavior - All tests now exercise actual production code paths - Better test coverage of the real functionality
The original implementation before PR #538 was much cleaner and simpler. This reverts to the elegant pop-first approach while keeping only the log verbosity fix. **What was reverted:** - Complex strategy manager backend selection - Search-and-remove logic with normalization - O(n) backend removal loops **What we kept:** - Simple pop-first approach: tryBackends[0] then tryBackends[1:] - Sequential order (predictable, no duplicates) - O(1) backend removal - Log verbosity fix (V(1) for failed backends) **Benefits of simple approach:** ✅ No duplicates - guaranteed by pop-first ✅ Tries all backends in order ✅ Clean, readable code ✅ Same behavior as before PR #538 **Tests updated:** - Simplified all tests to match the pop-first logic - Removed complex strategy manager interactions - Tests now verify sequential backend selection - All tests still pass and cover the actual logic This maintains the fix for issues #2 and #3 while being much simpler.
Connection refused errors are common when backends are down and should not spam the console at INFO level. This adds smart detection of connection refused errors in dialRoute to use verbosity 1 (debug level). Before: Connection refused → Verbosity 0 → INFO level → console spam After: Connection refused → Verbosity 1 → DEBUG level → quiet This preserves the smart verbosity system while fixing the specific case of connection refused errors that were causing spam.
Previously there were NO tests for individual strategy behaviors, only validation tests. This adds complete test coverage for all four load balancing strategies. Tests added: ✅ TestRandomStrategy - verifies random distribution ✅ TestRoundRobinStrategy - verifies sequential cycling ✅ TestRoundRobinStrategy_DifferentRoutes - verifies route isolation ✅ TestLeastConnectionsStrategy - verifies connection-based selection ✅ TestLowestLatencyStrategy - verifies latency-based selection ✅ TestStrategyWithEmptyBackends - edge case handling ✅ TestStrategyWithSingleBackend - single backend behavior ✅ TestGetNextBackendStrategyRouting - integration testing Each test verifies actual strategy behavior and ensures the algorithms work correctly according to their specifications.
Added explicit sequential strategy enum and made it the default behavior when no strategy is configured, providing clarity and consistency. Changes: ✅ Added StrategySequential enum to config ✅ Added sequentialNextBackend implementation ✅ Default empty strategy now uses sequential (not random) ✅ Updated documentation to reflect sequential as default ✅ Updated config files to list sequential as first option ✅ Added comprehensive tests for sequential strategy Strategy behavior: - Empty strategy → sequential (default) - strategy: sequential → explicit sequential - strategy: random → random selection - strategy: round-robin → round-robin cycling - strategy: least-connections → connection-based - strategy: lowest-latency → latency-based All existing strategies continue to work exactly as before. Sequential is now clearly documented and properly tested.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #564
Complete solution for console spam and strategy consistency in Gate Lite mode.
Issues Fixed ✅
1. Console Log Spam
dialRouteto use debug level (V=1) for connection refused2. Backend Cycling
3. Fallback Response
New Feature: Sequential Strategy ✨
Added Sequential Strategy Enum
sequentialstrategy as explicit optionStrategy Behavior
Technical Implementation
Smart Error Handling
Strategy Integration
Comprehensive Testing
Now includes complete test coverage:
Breaking Changes
None - All existing configurations continue to work:
Summary
This provides the complete solution with:
The fix addresses all reported issues while adding the missing sequential strategy enum and ensuring consistent, well-documented behavior.