bhouse-nexthop commented on PR #14109: URL: https://github.com/apache/cloudstack/pull/14109#issuecomment-5618788660
Thanks for the review, and for the approval @DaanHoogland — agreed that this wants real-world testing; it is opt-in behind `vm.allocation.algorithm=balancedweighted` precisely so it can be trialled on one cluster. On the Copilot comments: **`Long.MAX_VALUE` timestamp — fixed, and it was worse than described.** `vm_instance.update_time` is a `TIMESTAMP` column, so the ceiling is 2038-01-19, not the `DATETIME` 9999-12-31. `new Timestamp(Long.MAX_VALUE)` is roughly year 292 million. Rather than hunt for a sentinel that is both "far future" and representable, the query now omits the predicate entirely when no cut-off is given — the recent-count becomes a constant `0` and no timestamp is bound. **`catch (Throwable)` — changed to `catch (Exception)`.** Worth noting for context that this was copied from the neighbouring methods: `VMInstanceDaoImpl` has six other `catch (Throwable)` blocks, as does `CapacityDaoImpl`. I have only changed the one this PR adds, so as not to widen the diff. Happy to follow up with a separate cleanup across those files if that would be welcome. **Config key array formatting — done**, one entry per line. -- 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]
