kaisun2000 commented on a change in pull request #1302:
URL: https://github.com/apache/helix/pull/1302#discussion_r475975381



##########
File path: 
helix-core/src/test/java/org/apache/helix/integration/TestCustomizedViewAggregation.java
##########
@@ -410,7 +410,11 @@ public void testCustomizedViewAggregation() throws 
Exception {
     // Aggregating: Type A
     // Routing table: Type A, Type B, Type C
     setAggregationEnabledTypes(Arrays.asList(CustomizedStateType.TYPE_A));
-    validateAggregationSnapshot();
+    // This is commented out as a work around to pass the test
+    // The validation of config change will be done combined with the next 
several customized state changes
+    // The next validation should only show TYPE_A states aggregated in 
customized view
+    // Until we fix the issue in routing table provider 
https://github.com/apache/helix/issues/1296
+//    validateAggregationSnapshot();

Review comment:
       Sync-ed offline. 
   
   It seems that currentState based RP has one callbackhandler with all 
currentstate as child znode. Thus reset would refresh all currentstate only 
once. This is fine. 
   CustomizedView for each type has one callbackHandler. Thus, one removal of 
callbackhandler parent path would cause other types of customizedView be 
reset() too. 
   
   In sum, current design of routingProvider accomodate only one 
callbackhandler. (External view, currentstate). However customized view RP has 
callbackhandler per type. This does not fit into the current design of RP. 




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to