jiajunwang commented on a change in pull request #1154:
URL: https://github.com/apache/helix/pull/1154#discussion_r455993140
##########
File path:
helix-core/src/main/java/org/apache/helix/spectator/RoutingTableProvider.java
##########
@@ -413,6 +413,31 @@ public void addRoutingTableChangeListener(
_routingTableChangeListenerMap.put(routingTableChangeListener, new
ListenerContext(context));
logger.info("Attach RoutingTableProviderChangeListener {}",
routingTableChangeListener.getClass().getName());
+ if (_sourceDataTypeMap.isEmpty()) {
Review comment:
FYI, in Helix Manager, adding the listener will cause the listener to
get a "free" callback when the callback handler is init. So the requirement
here is doing the same for router.
Given that saying, if the routing table has not been updated (maybe during
the reading) then we return an empty snapshot. In this case, if the router
still notifies the listener once the update is done, then we are still good.
However, based on the code here, I think we might have a race condition that
this notification could be sent later than the proper one. Then user would get
the wrong information.
So we need a concurrency control here.
##########
File path:
helix-core/src/main/java/org/apache/helix/spectator/RoutingTableProvider.java
##########
@@ -413,6 +413,31 @@ public void addRoutingTableChangeListener(
_routingTableChangeListenerMap.put(routingTableChangeListener, new
ListenerContext(context));
logger.info("Attach RoutingTableProviderChangeListener {}",
routingTableChangeListener.getClass().getName());
+ if (_sourceDataTypeMap.isEmpty()) {
+
routingTableChangeListener.onRoutingTableChange(getRoutingTableSnapshot(),
context);
+ } else {
+ for (PropertyType propertyType : _sourceDataTypeMap.keySet()) {
Review comment:
As Junkai mentioned, I think we need a new interface for the listeners
anyway.
The current one won't tell the provider which type the listener wants to
listen to. So I think we just add a new API so as to add the listeners with the
right type.
In this case, the old API needs to be deprecated and if user add their
listen using this API, we shall just assign it to all the valid types.
----------------------------------------------------------------
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]
