perf(queue): skip unused server status checks - #246
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe change derives monitored servers from enabled availability-dependent features. Queue activity no longer depends on target availability when pausing is disabled. Unneeded server status data is removed before checks are scheduled. ChangesOnline server check selection
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Config
participant PistonQueuePlugin
participant ServerStatusManager
Config->>PistonQueuePlugin: provide serversRequiringOnlineChecks
PistonQueuePlugin->>ServerStatusManager: retainServers(required servers)
PistonQueuePlugin->>ServerStatusManager: schedule checks for required servers
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR reduces unnecessary server status polling by computing the set of servers that actually need online checks based on which availability features are enabled, and ensures stale server-status entries are cleared when checks are no longer needed.
Changes:
- Add
Config.serversRequiringOnlineChecks()to derive the exact server set needed for availability features. - Update scheduled tasks to skip status checks entirely when no feature needs them, and to stop position messages from depending on target status when queue pausing is disabled.
- Add
ServerStatusManager.retainServers(...)plus tests covering disabled checks and stale status cleanup.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| shared/src/main/java/net/pistonmaster/pistonqueue/shared/config/Config.java | Introduces derived server set for online checks based on enabled features. |
| shared/src/main/java/net/pistonmaster/pistonqueue/shared/plugin/PistonQueuePlugin.java | Uses the derived server set to skip unnecessary checks and avoid gating position messages when pausing is off. |
| shared/src/main/java/net/pistonmaster/pistonqueue/shared/queue/ServerStatusManager.java | Adds ability to remove stale status entries for servers no longer checked. |
| shared/src/test/java/net/pistonmaster/pistonqueue/shared/config/ConfigTest.java | Adds unit coverage for derived server-check set behavior under different feature toggles. |
| shared/src/test/java/net/pistonmaster/pistonqueue/shared/queue/logic/ServerStatusManagerTest.java | Adds unit coverage for stale status removal via retainServers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
73
to
+77
| final QueueGroup defaultGroup = config.getDefaultGroup(); | ||
| // Sends the position message and updates tab on an interval in chat | ||
| schedule(() -> { | ||
| boolean targetsOnline = defaultGroup.targetServers().stream().anyMatch(queueListener.getServerStatusManager().getOnlineServers()::contains); | ||
| boolean targetsOnline = !config.pauseQueueIfTargetDown() | ||
| || defaultGroup.targetServers().stream().anyMatch(queueListener.getServerStatusManager().getOnlineServers()::contains); |
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.
Summary
Production behavior
The current 6b6t production proxy configuration has both
pauseQueueIfTargetDownandkickWhenDowndisabled. With this change, its computed server check set is empty, so PistonQueue will stop pingingmain-serverandbackup-serverentirely.If either feature is enabled later, PistonQueue automatically checks only the servers that feature needs. No configuration migration or new option is required.
Testing
./gradlew spotlessApply build