alirezazamani commented on a change in pull request #834: Complete the Routing
Table Provider for CustomizedView
URL: https://github.com/apache/helix/pull/834#discussion_r391292345
##########
File path:
helix-core/src/main/java/org/apache/helix/spectator/RoutingDataCache.java
##########
@@ -66,31 +85,46 @@ public synchronized void refresh(HelixDataAccessor
accessor) {
super.refresh(accessor);
- if (_sourceDataType.equals(PropertyType.TARGETEXTERNALVIEW) &&
_propertyDataChangedMap
- .get(HelixConstants.ChangeType.TARGET_EXTERNAL_VIEW)) {
- long start = System.currentTimeMillis();
-
_propertyDataChangedMap.put(HelixConstants.ChangeType.TARGET_EXTERNAL_VIEW,
false);
- _targetExternalViewCache.refresh(accessor);
- LOG.info("Reload " +
_targetExternalViewCache.getExternalViewMap().keySet().size()
- + " TargetExternalViews. Takes " + (System.currentTimeMillis() -
start) + " ms");
- }
-
- if (_sourceDataType.equals(PropertyType.CURRENTSTATES) &&
_propertyDataChangedMap
- .get(HelixConstants.ChangeType.CURRENT_STATE)) {
- long start = System.currentTimeMillis();
- _propertyDataChangedMap.put(HelixConstants.ChangeType.CURRENT_STATE,
false);
- Map<String, LiveInstance> liveInstanceMap = getLiveInstances();
- _currentStateCache.refresh(accessor, liveInstanceMap);
- LOG.info("Reload CurrentStates. Takes " + (System.currentTimeMillis() -
start) + " ms");
+ switch (_sourceDataType) {
+ case TARGETEXTERNALVIEW:
+ if
(_propertyDataChangedMap.get(HelixConstants.ChangeType.TARGET_EXTERNAL_VIEW)) {
+ long start = System.currentTimeMillis();
+
_propertyDataChangedMap.put(HelixConstants.ChangeType.TARGET_EXTERNAL_VIEW,
false);
+ _targetExternalViewCache.refresh(accessor);
+ LOG.info("Reload " +
_targetExternalViewCache.getExternalViewMap().keySet().size()
+ + " TargetExternalViews. Takes " + (System.currentTimeMillis() -
start) + " ms");
+ }
+ break;
+ case CURRENTSTATES:
+ if
(_propertyDataChangedMap.get(HelixConstants.ChangeType.CURRENT_STATE)) {
+ long start = System.currentTimeMillis();
+ _propertyDataChangedMap.put(HelixConstants.ChangeType.CURRENT_STATE,
false);
+ Map<String, LiveInstance> liveInstanceMap = getLiveInstances();
+ _currentStateCache.refresh(accessor, liveInstanceMap);
+ LOG.info("Reload CurrentStates. Takes " + (System.currentTimeMillis()
- start) + " ms");
+ }
+ break;
+ case CUSTOMIZEDVIEW:
+ if
(_propertyDataChangedMap.get(HelixConstants.ChangeType.CUSTOMIZED_VIEW)
+ && !_aggregationType.isEmpty()) {
+ long start = System.currentTimeMillis();
+ _propertyDataChangedMap.put(HelixConstants.ChangeType.CUSTOMIZED_VIEW,
false);
+ _customizedViewCache.refresh(accessor);
+ LOG.info("Reload " +
_customizedViewCache.getCustomizedViewMap().keySet().size()
+ + " CustomizedView. Takes " + (System.currentTimeMillis() - start)
+ " ms");
+ }
+ break;
+ default:
+ break;
Review comment:
I think we shouldn't throw an exception here. If you look at the original
code (without my changes), we do not through an exception and we only refresh
for TargettedExternalView and CurrentState. Also since we do not have
ExternalView case here, we just want to ignore cases and continue with other
propertyTypes.
----------------------------------------------------------------
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]
