zhangmeng916 commented on a change in pull request #1745:
URL: https://github.com/apache/helix/pull/1745#discussion_r634661185
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java
##########
@@ -1185,6 +1191,18 @@ public void onClusterConfigChange(ClusterConfig
clusterConfig,
.info("END: GenericClusterController.onClusterConfigChange() for
cluster " + _clusterName);
}
+ @Override
+ @PreFetch(enabled = false)
+ public void onLiveInstanceDataChange(LiveInstance liveInstance,
NotificationContext context) {
+ logger.info("START: GenericClusterController.onLiveInstanceDataChange()
for cluster {}",
+ _clusterName);
+ // Selectively refresh live instances cache.
+ notifyCaches(context, ChangeType.LIVE_INSTANCE);
+ pushToEventQueues(ClusterEventType.LiveInstanceChange, context,
Collections.emptyMap());
Review comment:
Do we want to reuse the previous event type? If this is a new use case,
we can have a new event type to distinguish individual instance data change
from previous live instance children change.
##########
File path: helix-core/src/test/java/org/apache/helix/TestListenerCallback.java
##########
@@ -275,4 +279,29 @@ public void testScopedConfigChangeListener() throws
Exception {
"Should get resourceConfig callback invoked since we add
resourceConfig");
Assert.assertEquals(listener._configSize, 0, "Resource Config size does
not match");
}
+
+ @Test
+ public void testLiveInstanceDataChangeListener() {
+ // start 1 participant
+ String instanceName = "localhost_" + 12918;
+ MockParticipantManager participant =
+ new MockParticipantManager(ZK_ADDR, _clusterName, instanceName);
+ participant.syncStart();
+
+ HelixDataAccessor accessor = _manager.getHelixDataAccessor();
+ LiveInstance liveInstance =
+ accessor.getProperty(accessor.keyBuilder().liveInstance(instanceName));
+ AtomicBoolean liveInstanceDataChangeNotified = new AtomicBoolean(false);
+
+ // add live instance data change listener and update live instance
+ LiveInstanceDataChangeListener listener =
+ (liveInstanceChanged, notificationContext) ->
liveInstanceDataChangeNotified.set(true);
+ _manager.addLiveInstanceDataChangeListener(listener, instanceName);
+ accessor.updateProperty(accessor.keyBuilder().liveInstance(instanceName),
liveInstance);
+
+ Assert.assertTrue(liveInstanceDataChangeNotified.get());
Review comment:
I would suggest to use TestHelper.verify(() instead of directly "assert"
to validate the results so that the test is more stable. You can take a look at
other test cases in this file as an example.
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
##########
@@ -267,6 +269,13 @@ private void parseListenerProperties() {
break;
case CONTROLLER:
listenerClass = ControllerChangeListener.class;
+ break;
+ case LIVE_INSTANCE_DATA:
+ listenerClass = LiveInstanceDataChangeListener.class;
+ break;
+ default:
+ logger.warn("Unexpected change type {} in callback {}", _changeType,
_uid);
Review comment:
This is a new case. I think if we do arrive at this case, we should log
it as an error.
##########
File path: helix-core/src/main/java/org/apache/helix/HelixManager.java
##########
@@ -129,6 +132,15 @@
*/
void addLiveInstanceChangeListener(LiveInstanceChangeListener listener)
throws Exception;
+ /**
+ * @see
LiveInstanceDataChangeListener#onLiveInstanceDataChange(LiveInstance,
NotificationContext)
+ * @param listener {@link LiveInstanceDataChangeListener}
+ */
+ default void
addLiveInstanceDataChangeListener(LiveInstanceDataChangeListener listener,
+ String instanceName) {
+ throw new NotImplementedException("Not implemented");
Review comment:
This line is unnecessary, as following the pattern for most other
listeners in this class.
--
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]
