[Refactor] optimize code structure about collector and job within Manager #3675
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the code structure of the Manager module, specifically focusing on collector operations and job scheduling. The primary goal is to improve code organization and create a clearer separation of responsibilities within the CollectorJobScheduler component.
- Split interfaces to separate collector operations from job operations
- Introduced
CollectorKeeperabstraction for collector selection strategies - Moved properties classes to a dedicated
propertiespackage - Consolidated duplicate methods and simplified the API surface
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| CollectorJobScheduler.java | Refactored to implement split interfaces and use CollectorKeeper abstraction |
| JobOperation.java | New simplified interface replacing CollectJobScheduling |
| CollectorOperation.java | New interface for direct collector operations |
| CollectorOperationReceiver.java | New interface for receiving collector operations from remote collectors |
| ConsistentHashCollectorKeeper.java | Renamed and refactored from ConsistentHash, implements CollectorKeeper |
| CollectorKeeper.java | New interface abstraction for collector selection strategies |
| CollectorNode.java | Extracted from inner class to standalone POJO |
| JobCache.java | New utility class for job caching |
| properties/*.java | Moved property classes to dedicated package |
Comments suppressed due to low confidence (3)
hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/scheduler/ConsistentHashCollectorKeeper.java:302
- Potential null pointer exception: The addJob method can return null when no collector is found, but the code immediately uses collectorNode.getIdentity() without null checking.
Map.Entry<Integer, CollectorNode> ceilEntry = hashCircle.ceilingOrFirstEntry(dispatchHash);
hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/nativex/HertzbeatRuntimeHintsRegistrar.java:29
- Unused import: org.springframework.aot.hint.ExecutableMode is imported but not used after the registerConstructor method was removed.
import org.apache.sshd.common.util.security.eddsa.EdDSASecurityProviderRegistrar;
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/pojo/JobCache.java
Show resolved
Hide resolved
…ojo/JobCache.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Calvin <zhengqiwei@apache.org>
|
hi, this need more time or others to review and test , maybe in this weekend. |
Sure, looking forward to your suggestions |
What's changed?
After code iterations in these years, module
Managercontain more and more functions. Yet there isn't a clear code structure, such asorg.apache.hertzbeat.manager.scheduler.CollectorJobSchedulerholds all operations about Collector status notifications, selecting collector, dispatching jobs and so on.I think it is time to get refactored.
ConfigConstants.PkgConstant.PKGinstead of hardcode inManager.javapropertiesin moduleManager, used for those files that read yml properties onlyorg.apache.hertzbeat.manager.scheduler.CollectorJobScheduler#collectSyncJobDataandorg.apache.hertzbeat.manager.scheduler.CollectorJobScheduler#updateAsyncCollectJoborg.apache.hertzbeat.manager.scheduler.CollectJobScheduling->org.apache.hertzbeat.manager.scheduler.JobOperationorg.apache.hertzbeat.manager.scheduler.CollectorScheduling->org.apache.hertzbeat.manager.scheduler.CollectorOperationandorg.apache.hertzbeat.manager.scheduler.CollectorOperationReceiverCollectorKeeperto take responsibility of collector selections onlyAfter refactoring above, current code design of Manager will be like:
So far, we keep
org.apache.hertzbeat.manager.scheduler.CollectorJobScheduleras the only one implementation class in order to avoid overturning.What are the benefits?
org.apache.hertzbeat.manager.scheduler.CollectorScheduling-> Ensure the code hierarchy clearer to facilitate further code splitting.CollectorKeeper-> This is for collector selections only, which means we can support more select strategy by implementing this interface, such asRound Robinand so on.Checklist
Add or update API