sergehuber commented on PR #931: URL: https://github.com/apache/unomi/pull/931#issuecomment-6453410430
Looks good to me, CI is green. A few minor, non-blocking points: 1. `HealthCheckService.refreshCache()`: the generic `catch (Exception e)` around `future.get(...)` also catches `InterruptedException`. The interrupt flag is lost and the remaining futures are not cancelled. A separate catch that restores the flag and cancels them would fix it. 2. The same catch logs `"Error while executing health check"` without the provider name. The timeout branch has it. 3. Could the executor use a named thread factory (e.g. `unomi-healthcheck-%d`)? `pool-N-thread-M` is hard to spot in a thread dump. 4. No test covers the new `isCacheExpired()` check inside `cacheLock`, e.g. two threads on an expired cache, provider called once. Out of scope: the ES/OpenSearch providers cache a LIVE result for 10s, so a hang right after a LIVE check still shows LIVE for up to 10s. Maybe worth a note on UNOMI-1025. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
