btlqql commented on PR #4710:
URL: 
https://github.com/apache/rocketmq-dashboard/pull/4710#issuecomment-5761964493

   Reworked onto the current `rocketmq-studio` head, and you were right on the 
mechanism — I could not produce a padded `rmq_system_alert.instance_id` either:
   
   - the only writers of that table are `NativeAlertEvaluationService.evaluate` 
(`:87`) and `NativeAlertProcessor.emitLifecycleEvent` (`:188`), and both take 
`MetricSample.instanceId()`;
   - every sample in the collectors is built from `instance.getName()`, and 
every writer of `rmq_instance.name` trims it (`InstanceService.java:468`, 
`:508` via `requireInstanceName`, including the cloud-import path at `:506`).
   
   So the description no longer claims a reachable row. What the PR does now is 
remove the asymmetry itself, and it covers the second site you named: 
`AlertService.java:506`, where the Java comparison is the only filter, because 
`findAlerts(level)` applies no instance filter in SQL. The fixture is fixed too 
— trailing padding only, with the collation note (MySQL 8's 
`utf8mb4_0900_ai_ci` is NO PAD, which is exactly why the earlier leading-space 
fixture pinned a state the query cannot return).
   
   Full suite on a host with MySQL 8 reachable: 3147 tests, 0 failures, 0 
errors (3145 on the untouched head).
   
   If you would rather the alert path were left as it is than made consistent 
with #4464's rule, say so and I will close this one — I would rather withdraw 
it than argue a no-op into the branch.
   


-- 
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]

Reply via email to