jiajunwang commented on a change in pull request #834: Complete the Routing
Table Provider for CustomizedView
URL: https://github.com/apache/helix/pull/834#discussion_r394036351
##########
File path:
helix-core/src/main/java/org/apache/helix/spectator/RoutingTableProvider.java
##########
@@ -280,17 +425,50 @@ public Object removeRoutingTableChangeListener(
}
/**
- * returns the instances for {resource,partition} pair that are in a specific
- * {state}
+ * For specific routing table associated with {propertyType, stateType}
+ * returns the instances for {resource group,partition} pair contains any of
the given tags that
+ * are in a specific {state}.
+ * @param resourceName
+ * @param partitionName
+ * @param state
+ * @param propertyType
+ * @param stateType
+ * @return
+ */
+ public List<InstanceConfig> getInstances(String resourceName, String
partitionName, String state,
+ PropertyType propertyType, String stateType) {
+ return getInstancesForResource(resourceName, partitionName, state,
propertyType, stateType);
+ }
+
+ /**
+ * returns the instances for {resource,partition} pair that are in a
specific {state}
* @param resourceName
- * -
* @param partitionName
* @param state
* @return empty list if there is no instance in a given state
*/
public List<InstanceConfig> getInstancesForResource(String resourceName,
String partitionName,
String state) {
- return _routingTableRef.get().getInstancesForResource(resourceName,
partitionName, state);
+ String key = getRoutingTableKey(DEFAULT_PROPERTY_TYPE, DEFAULT_STATE_TYPE);
+ return
_routingTableRefMap.get(key).get().getInstancesForResource(resourceName,
partitionName,
+ state);
+ }
+
+ /**
+ * For specific routing table associated with {propertyType, stateType}
+ * returns the instances for {resource,partition} pair that are in a
specific {state}
+ * @param resourceName
+ * @param partitionName
+ * @param state
+ * @param propertyType
+ * @param stateType
+ * @return
+ */
+ public List<InstanceConfig> getInstancesForResource(String resourceName,
String partitionName,
Review comment:
I'm still concerned about these get methods. I reviewed the design, it seems
that we don't have a detailed list of what should be returned in the provider.
So let's be strict on these public methods. My feeling is that we don't need
all these additional methods. Since our application clients would use the
snapshot object to get the info they need.
On the other hand, if the method is for testing only, try to keep them
protected (and make the test case located in the same package path). Or we can
leave only one method to get the detailed customized state map for testing.
Note that once you add the public methods, we should not remove them later.
Without solid business logic usage, I don't think we should add these
additional methods.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
