zhangmeng916 commented on pull request #1307:
URL: https://github.com/apache/helix/pull/1307#issuecomment-679421854


   > getClusterTopologySetting
   
   The big problem I have using Topology is that the logic handling is 
different. If I'm going to use it, I'll need to introduce a lot of switch 
cases, as the main purpose for Topology is for rebalance instead of API 
response to users. Here're a few examples:
   1. if topology is null, it does
   clusterTopologyConfig.endNodeType = Types.INSTANCE.name();
   clusterTopologyConfig.faultZoneType = Types.ZONE.name();
   But I'll throw an exception and return.
   
   2. When an instance topology misses a value, it also fills the default 
value, but I will need to throw an exception. 
   
   3. When you find an instance topology is not defined, you'll throw an 
exception of terminating rebalance. This is not something I should have in the 
log and return to users.
   
   4. With current clusterTopologyConfig.topologyKeyDefaultValue's 
linkedhashmap, it is not as easy/efficient as a trie to return the topology 
under each domain. 
   
   In all, the logic of Topology is more specific for rebalancing usage. If I 
change it, it will become a complicated class that needs to adapt to different 
usage. I think the correct way in the future should be:
   Have a base class that returns the raw data in cluster config and instance 
config, and different classes extend it for different usage. 
   


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