The current implementation mixes state evaluation, persistence, and transaction management across multiple classes (MicroService, MonitorServices, DockerService), which has lead to both errors being ignored silently and service states being reported incorrectly. Additionally, a complex control flow complicates debugging.
Proposals:
- Separate concerns: determine state first (regardless of chosen method), call persisting method afterwards and only once
- Centralize transaction handling: reuse RestTemplate via injection, optionally use
@Retryable to simplify retry logic (e.g. @Retryable(value = CannotAcquireLockException.class, maxAttempts = 3)), avoid creating transactions in helper methods such as getMonService() which has lead to broken transactions in the past, or implement proper transaction propagation
- Simplify control flow: split check scenarios into smaller methods with proper log messages to simplify debugging
- Guard against empty optionals (MicroService)
- Optionally, replace Thread with Spring's
@Scheduled: managed by Spring container, automatic lifecycle, built-in scheduling options, cleaner code
The current implementation mixes state evaluation, persistence, and transaction management across multiple classes (MicroService, MonitorServices, DockerService), which has lead to both errors being ignored silently and service states being reported incorrectly. Additionally, a complex control flow complicates debugging.
Proposals:
@Retryableto simplify retry logic (e.g.@Retryable(value = CannotAcquireLockException.class, maxAttempts = 3)), avoid creating transactions in helper methods such asgetMonService()which has lead to broken transactions in the past, or implement proper transaction propagation@Scheduled: managed by Spring container, automatic lifecycle, built-in scheduling options, cleaner code