You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by "zpinto (via GitHub)" <gi...@apache.org> on 2023/12/06 01:44:32 UTC
[PR] Prevent the spectator routing table from containing SWAP_IN instances [helix]
zpinto opened a new pull request, #2710:
URL: https://github.com/apache/helix/pull/2710
### Issues
- [x] not populate SWAP_IN replicas in routing tables until SWAP is completed to avoid spectator serving traffic for replicas on the swapping in instance. #2662
### Description
During a swap operation, we are essentially creating a mirror node which is labeled with the SWAP_IN instance operation. Until the point that the swap operation is complete we do not want to serve traffic to this node. To achieve this, the RoutingDataCache will have a notion of routableInstanceConfigs and routableLiveInstances.
Only these nodes are used to create the routingTables. At this time, only SWAP_IN instances are considered not to be routable.
### Tests
- [x] Updated the swap tests in TestInstanceOperation to include starting a spectator and validating the EV, CS, and Default routing tables.
All TestInstanceOperation tests are passing.
### Changes that Break Backward Compatibility (Optional)
NA
### Commits
- My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Code Quality
- My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
Re: [PR] Prevent the spectator routing table from containing SWAP_IN instances [helix]
Posted by "xyuanlu (via GitHub)" <gi...@apache.org>.
xyuanlu merged PR #2710:
URL: https://github.com/apache/helix/pull/2710
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
Re: [PR] Prevent the spectator routing table from containing SWAP_IN instances [helix]
Posted by "zpinto (via GitHub)" <gi...@apache.org>.
zpinto commented on PR #2710:
URL: https://github.com/apache/helix/pull/2710#issuecomment-1846429496
CI passed except for testCacheDataUpdates which is a flaky test.
This PR is ready to be merged.
Final Commit Message:
Prevent the spectator routing table from containing SWAP_IN instances.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
Re: [PR] Prevent the spectator routing table from containing SWAP_IN instances [helix]
Posted by "zpinto (via GitHub)" <gi...@apache.org>.
zpinto commented on code in PR #2710:
URL: https://github.com/apache/helix/pull/2710#discussion_r1419842674
##########
helix-core/src/main/java/org/apache/helix/spectator/RoutingDataCache.java:
##########
@@ -150,6 +181,41 @@ public synchronized void refresh(HelixDataAccessor accessor) {
}
}
+ private void updateRoutableInstanceConfigMap(Map<String, InstanceConfig> instanceConfigMap) {
+ _routableInstanceConfigMap = instanceConfigMap.entrySet().stream().filter(
+ (instanceConfigEntry) -> !NON_ROUTABLE_INSTANCE_OPERATIONS.contains(
+ instanceConfigEntry.getValue().getInstanceOperation()))
+ .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+ }
+
+ private void updateRoutableLiveInstanceMap(Map<String, InstanceConfig> instanceConfigMap,
+ Map<String, LiveInstance> liveInstanceMap) {
+ _routableLiveInstanceMap = liveInstanceMap.entrySet().stream().filter(
+ (liveInstanceEntry) -> instanceConfigMap.containsKey(liveInstanceEntry.getKey())
+ && !NON_ROUTABLE_INSTANCE_OPERATIONS.contains(
+ instanceConfigMap.get(liveInstanceEntry.getKey()).getInstanceOperation()))
+ .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+ }
+
+ /**
+ * Returns the LiveInstances for each of the routable instances that are currently up and
+ * running.
+ *
+ * @return a map of LiveInstances
+ */
+ public Map<String, LiveInstance> getRoutableLiveInstances() {
+ return Collections.unmodifiableMap(_routableLiveInstanceMap);
+ }
+
+ /**
+ * Returns the instance config map for all the routable instances that are in the cluster.
+ *
+ * @return a map of InstanceConfigs
+ */
+ public Map<String, InstanceConfig> getRoutableInstanceConfigMap() {
Review Comment:
We have usage of this in RoutingTableProvider.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
Re: [PR] Prevent the spectator routing table from containing SWAP_IN instances [helix]
Posted by "junkaixue (via GitHub)" <gi...@apache.org>.
junkaixue commented on code in PR #2710:
URL: https://github.com/apache/helix/pull/2710#discussion_r1419685548
##########
helix-core/src/main/java/org/apache/helix/spectator/RoutingDataCache.java:
##########
@@ -150,6 +181,41 @@ public synchronized void refresh(HelixDataAccessor accessor) {
}
}
+ private void updateRoutableInstanceConfigMap(Map<String, InstanceConfig> instanceConfigMap) {
+ _routableInstanceConfigMap = instanceConfigMap.entrySet().stream().filter(
+ (instanceConfigEntry) -> !NON_ROUTABLE_INSTANCE_OPERATIONS.contains(
+ instanceConfigEntry.getValue().getInstanceOperation()))
+ .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+ }
+
+ private void updateRoutableLiveInstanceMap(Map<String, InstanceConfig> instanceConfigMap,
+ Map<String, LiveInstance> liveInstanceMap) {
+ _routableLiveInstanceMap = liveInstanceMap.entrySet().stream().filter(
+ (liveInstanceEntry) -> instanceConfigMap.containsKey(liveInstanceEntry.getKey())
+ && !NON_ROUTABLE_INSTANCE_OPERATIONS.contains(
+ instanceConfigMap.get(liveInstanceEntry.getKey()).getInstanceOperation()))
+ .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+ }
+
+ /**
+ * Returns the LiveInstances for each of the routable instances that are currently up and
+ * running.
+ *
+ * @return a map of LiveInstances
+ */
+ public Map<String, LiveInstance> getRoutableLiveInstances() {
+ return Collections.unmodifiableMap(_routableLiveInstanceMap);
+ }
+
+ /**
+ * Returns the instance config map for all the routable instances that are in the cluster.
+ *
+ * @return a map of InstanceConfigs
+ */
+ public Map<String, InstanceConfig> getRoutableInstanceConfigMap() {
Review Comment:
If we don't have the usage of this, maybe let's remove it.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org