himanshukandwal commented on code in PR #2829:
URL: https://github.com/apache/helix/pull/2829#discussion_r1669475561
##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/constraints/ConstraintBasedAlgorithm.java:
##########
@@ -120,24 +121,24 @@ public OptimalAssignment calculate(ClusterModel
clusterModel) throws HelixRebala
private Optional<AssignableNode> getNodeWithHighestPoints(AssignableReplica
replica,
List<AssignableNode> assignableNodes, ClusterContext clusterContext,
Set<String> busyInstances, OptimalAssignment optimalAssignment) {
- Map<AssignableNode, List<HardConstraint>> hardConstraintFailures = new
ConcurrentHashMap<>();
+ Map<AssignableNode, List<String>> hardConstraintFailures = new
ConcurrentHashMap<>();
Review Comment:
Currently, `List<HardConstraint>>` just contains the reference of the
hardConstraint singleton object and the error message is just a static string,
which is not useful.
For example: `NodeCapacityConstraint`, We log: ` "Node has insufficient
capacity"` which seems not useful enough as we dont get which key is failing
for capacity and by how much. We have this as part of debug log but with many
consumer now onboarded to WAGED and hard constraint issue happening fairly
often so its better that its part of log (error) message itself. Also, we dont
want to print it always, there are false positive as well, and only when there
is a total failure.
--
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: reviews-unsubscr...@helix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
