[GitHub] [helix] nikhilvavs commented on issue #748: Huge CPU spike after upgrading from 0.8.2 to 0.8.4 (0.8.3)
nikhilvavs commented on issue #748: Huge CPU spike after upgrading from 0.8.2 to 0.8.4 (0.8.3) URL: https://github.com/apache/helix/issues/748#issuecomment-602420386 I tested once again and found that this started in 0.8.3. The flame graph showed that it was taking a lot of time in the deserializer, but I don't see any major changes that can cause this b/w 0.8.2 and 0.8.3 - [Diff](https://github.com/apache/helix/compare/helix-0.8.2...helix-0.8.3?diff=unified) I would be happy to help debug this further, but if you can give me some steps that I can take. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] nikhilvavs edited a comment on issue #748: Huge CPU spike after upgrading from 0.8.2 to 0.8.4 (0.8.3)
nikhilvavs edited a comment on issue #748: Huge CPU spike after upgrading from 0.8.2 to 0.8.4 (0.8.3) URL: https://github.com/apache/helix/issues/748#issuecomment-602420386 I tested once again and found that this started in 0.8.3. The flame graph showed that it was taking a lot of time in the deserializer, but I don't see any major changes that can cause this b/w 0.8.2 and 0.8.3 - [Diff](https://github.com/apache/helix/compare/helix-0.8.2...helix-0.8.3?diff=unified) I would be happy to help debug this further, it would be great if you can give me some steps that I can take. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiminoc opened a new issue #905: Helix Agent document out of sync with code?
jiminoc opened a new issue #905: Helix Agent document out of sync with code? URL: https://github.com/apache/helix/issues/905 I tried following the tutorial for the Helix Agent where it says to use the following command line configuration values to set state changes. ` # Specify the script for OFFLINE --> ONLINE /helix-admin.sh --zkSvr localhost:2181 --setConfig CLUSTER clusterName OFFLINE-ONLINE.command="simpleHttpClient.py OFFLINE-ONLINE",OFFLINE-ONLINE.workingDir="/path/to/script", OFFLINE-ONLINE.command.pidfile="/path/to/pidfile" ` based on my testing and inspection of the code the working directory parameter seemed to have wanted this instead ` OFFLINE-ONLINE.command.workingDir="/path/to/script", ` see the added "command" slug in there. Once I added that I was able to get the agent executing the example client python script reference page: https://helix.apache.org/0.9.4-docs/tutorial_agent.html 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiminoc opened a new issue #906: Helix Agent doc examples not parsing command line parameters correctly
jiminoc opened a new issue #906: Helix Agent doc examples not parsing command line parameters correctly URL: https://github.com/apache/helix/issues/906 I tried following the tutorial here: https://helix.apache.org/0.9.4-docs/tutorial_agent.html specifically when I tried to add state changes listed below `./helix-admin.sh --zkSvr 10.148.110.208:2181 --setConfig CLUSTER LIVENESS ONLINE-SLAVE.command="simpleHttpClient.py ONLINE-SLAVE"` when I look in the Helix Admin UI, the configuration section only shows `simpleHttpClient.py` as the parameter for _ONLINE-SLAVE.command_ I went into the method: `parseCsvFormatedKeyValuePairs` in _HelixUtil.java_ and changed `String[] pairs = keyValuePairs.split("[\\s,]");` to `String[] pairs = keyValuePairs.split(",");` and it works correcly. When I rebuild and run again with that change and look in Helix UI the configuration then shows `simpleHttpClient.py ONLINE-SLAVE` and my python example script receives the correct parameters after that. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396824926 ## File path: helix-core/src/main/java/org/apache/helix/api/listeners/CustomizedStateRootChangeListener.java ## @@ -0,0 +1,36 @@ +package org.apache.helix.api.listeners; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import org.apache.helix.NotificationContext; + + +/** + * Interface to implement to respond to changes in the root path of customized state + */ +public interface CustomizedStateRootChangeListener { + + /** + * Invoked when root path customized state changes + * @param instanceName name of the instance whose state changed + * @param changeContext the change event and state + */ + void onCustomizedStateRootChange(String instanceName, NotificationContext changeContext); Review comment: Shall this method be passed a list of types in case anyone defined it with PreFetch == true? 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396796706 ## File path: helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java ## @@ -775,6 +811,39 @@ public void onStateChange(String instanceName, List statesInfo, logger.info("END: GenericClusterController.onStateChange()"); } + @Override + @PreFetch(enabled = false) + public void onCustomizedStateRootChange(String instanceName, NotificationContext changeContext) { +logger.info("START: GenericClusterController.onCustomizedStateRootChange()"); +notifyCaches(changeContext, ChangeType.CUSTOMIZED_STATE_ROOT); +HelixManager manager = changeContext.getManager(); +List customizedStateTypes = +manager.getHelixDataAccessor().getChildNames( + manager.getHelixDataAccessor().keyBuilder().customizedStatesRoot(instanceName)); + +for (String customizedState : customizedStateTypes) { Review comment: Shall we do the deletion here as well? 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396800481 ## File path: helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java ## @@ -259,45 +266,52 @@ private void parseListenerProperties() { Class listenerClass = null; switch (_changeType) { -case IDEAL_STATE: - listenerClass = IdealStateChangeListener.class; - break; -case INSTANCE_CONFIG: - if (_listener instanceof ConfigChangeListener) { + case IDEAL_STATE: +listenerClass = IdealStateChangeListener.class; +break; + case INSTANCE_CONFIG: +if (_listener instanceof ConfigChangeListener) { + listenerClass = ConfigChangeListener.class; +} else if (_listener instanceof InstanceConfigChangeListener) { + listenerClass = InstanceConfigChangeListener.class; +} +break; + case CLUSTER_CONFIG: +listenerClass = ClusterConfigChangeListener.class; +break; + case RESOURCE_CONFIG: +listenerClass = ResourceConfigChangeListener.class; +break; + case CUSTOMIZED_STATE_CONFIG: +listenerClass = CustomizedStateConfigChangeListener.class; +break; + case CONFIG: listenerClass = ConfigChangeListener.class; - } else if (_listener instanceof InstanceConfigChangeListener) { -listenerClass = InstanceConfigChangeListener.class; - } - break; -case CLUSTER_CONFIG: - listenerClass = ClusterConfigChangeListener.class; - break; -case RESOURCE_CONFIG: - listenerClass = ResourceConfigChangeListener.class; - break; -case CONFIG: - listenerClass = ConfigChangeListener.class; - break; -case LIVE_INSTANCE: - listenerClass = LiveInstanceChangeListener.class; - break; -case CURRENT_STATE: - listenerClass = CurrentStateChangeListener.class; - ; - break; -case MESSAGE: -case MESSAGES_CONTROLLER: - listenerClass = MessageListener.class; - break; -case EXTERNAL_VIEW: -case TARGET_EXTERNAL_VIEW: - listenerClass = ExternalViewChangeListener.class; - break; -case CUSTOMIZED_VIEW: - listenerClass = CustomizedViewChangeListener.class; - break; -case CONTROLLER: - listenerClass = ControllerChangeListener.class; +break; + case LIVE_INSTANCE: +listenerClass = LiveInstanceChangeListener.class; +break; + case CURRENT_STATE: +listenerClass = CurrentStateChangeListener.class; +break; + case CUSTOMIZED_STATE_ROOT: +listenerClass = CustomizedStateRootChangeListener.class; Review comment: This can also use the same listener class if you move the handling method to CustomizedStateChangeListener. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396797454 ## File path: helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java ## @@ -1233,4 +1334,18 @@ private void initPipeline(Thread eventThread, BaseControllerDataProvider cache) eventThread.setDaemon(true); eventThread.start(); } -} \ No newline at end of file + + private List getEnabledCustomizedStates(HelixManager manager) { Review comment: It seems there is no usage of this private method. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396797274 ## File path: helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java ## @@ -1233,4 +1334,18 @@ private void initPipeline(Thread eventThread, BaseControllerDataProvider cache) eventThread.setDaemon(true); eventThread.start(); } -} \ No newline at end of file + + private List getEnabledCustomizedStates(HelixManager manager) { +CustomizedStateConfig customizedStateConfig = +manager.getConfigAccessor().getCustomizedStateConfig(_clusterName); +if (customizedStateConfig != null) { + return customizedStateConfig.getAggregationEnabledTypes(); +} +return Collections.emptyList(); + } + + private void addCustomizedStateListeners(HelixManager manager, String customizedState, Review comment: What is this? 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396652209 ## File path: helix-core/src/main/java/org/apache/helix/PropertyKey.java ## @@ -474,6 +474,15 @@ public PropertyKey currentState(String instanceName, String sessionId, String re } } +/** + * Get a property key associated with the root of {@link CustomizedState} of an instance + * @param instanceName + * @return {@link PropertyKey} + */ +public PropertyKey customizedStatesRoot(String instanceName) { Review comment: I would prefer to call this method customizedState**s**, and the one for the specific state customizedState(without the s) 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396799467 ## File path: helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java ## @@ -775,6 +811,39 @@ public void onStateChange(String instanceName, List statesInfo, logger.info("END: GenericClusterController.onStateChange()"); } + @Override + @PreFetch(enabled = false) + public void onCustomizedStateRootChange(String instanceName, NotificationContext changeContext) { +logger.info("START: GenericClusterController.onCustomizedStateRootChange()"); +notifyCaches(changeContext, ChangeType.CUSTOMIZED_STATE_ROOT); Review comment: We don't need to notify the cache, right? 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396824077 ## File path: helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java ## @@ -424,6 +446,20 @@ public void invoke(NotificationContext changeContext) throws Exception { List currentStates = preFetch(_propertyKey); currentStateChangeListener.onStateChange(instanceName, currentStates, changeContext); + } else if (_changeType == CUSTOMIZED_STATE_ROOT) { +CustomizedStateRootChangeListener customizedStateRootChangeListener = +(CustomizedStateRootChangeListener) _listener; +String instanceName = PropertyPathConfig.getInstanceNameFromPath(_path); + customizedStateRootChangeListener.onCustomizedStateRootChange(instanceName, Review comment: If prefetch, I guess we should get all the children for types and pass to the onCustomizedStateRootChange() method? 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
jiajunwang commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396824725 ## File path: helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java ## @@ -569,12 +570,19 @@ public void addCurrentStateChangeListener(org.apache.helix.CurrentStateChangeLis }); } + @Override + public void addCustomizedStateRootChangeListener(CustomizedStateRootChangeListener listener, + String instanceName) throws Exception { +addListener(listener, new Builder(_clusterName).customizedStatesRoot(instanceName), +ChangeType.CUSTOMIZED_STATE_ROOT, new EventType[]{EventType.NodeChildrenChanged}); + } + @Override public void addCustomizedStateChangeListener(CustomizedStateChangeListener listener, - String instanceName, String customizedStateName) throws Exception { -addListener(listener, new Builder(_clusterName).customizedStates(instanceName, customizedStateName), -ChangeType.CUSTOMIZED_STATE, new EventType[] { EventType.NodeChildrenChanged -}); + String instanceName, String customizedStateType) throws Exception { Review comment: That would be one additional layer than the current callbackhandler infrastructure can support. I would like to have a try to improve it after this PR is in. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] alirezazamani removed a comment on issue #869: Replace customized view cache with property cache
alirezazamani removed a comment on issue #869: Replace customized view cache with property cache URL: https://github.com/apache/helix/pull/869#issuecomment-595448191 This PR is depending on PR #834 and only the last commit is new compared to the other PR. Please do not review it until other one in merged. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] zhangmeng916 commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
zhangmeng916 commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396867483 ## File path: helix-core/src/main/java/org/apache/helix/PropertyKey.java ## @@ -474,6 +474,15 @@ public PropertyKey currentState(String instanceName, String sessionId, String re } } +/** + * Get a property key associated with the root of {@link CustomizedState} of an instance + * @param instanceName + * @return {@link PropertyKey} + */ +public PropertyKey customizedStatesRoot(String instanceName) { Review comment: This might cause more confusion as there're many places that have customizedStates as variable name. I fear it actually makes the later modification (combine two layers) more difficult. Have a "root" name here can help us know exactly where need to be changed. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] zhangmeng916 commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
zhangmeng916 commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396866721 ## File path: helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java ## @@ -569,12 +570,19 @@ public void addCurrentStateChangeListener(org.apache.helix.CurrentStateChangeLis }); } + @Override + public void addCustomizedStateRootChangeListener(CustomizedStateRootChangeListener listener, + String instanceName) throws Exception { +addListener(listener, new Builder(_clusterName).customizedStatesRoot(instanceName), +ChangeType.CUSTOMIZED_STATE_ROOT, new EventType[]{EventType.NodeChildrenChanged}); + } + @Override public void addCustomizedStateChangeListener(CustomizedStateChangeListener listener, - String instanceName, String customizedStateName) throws Exception { -addListener(listener, new Builder(_clusterName).customizedStates(instanceName, customizedStateName), -ChangeType.CUSTOMIZED_STATE, new EventType[] { EventType.NodeChildrenChanged -}); + String instanceName, String customizedStateType) throws Exception { Review comment: Yeah, agree. @jiajunwang and I will look at how to merge the two layers later. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] zhangmeng916 commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation
zhangmeng916 commented on a change in pull request #851: Modify Helix generic controller to include new stage for customized view aggregation URL: https://github.com/apache/helix/pull/851#discussion_r396866020 ## File path: helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java ## @@ -259,45 +266,52 @@ private void parseListenerProperties() { Class listenerClass = null; switch (_changeType) { -case IDEAL_STATE: - listenerClass = IdealStateChangeListener.class; - break; -case INSTANCE_CONFIG: - if (_listener instanceof ConfigChangeListener) { + case IDEAL_STATE: +listenerClass = IdealStateChangeListener.class; +break; + case INSTANCE_CONFIG: +if (_listener instanceof ConfigChangeListener) { + listenerClass = ConfigChangeListener.class; +} else if (_listener instanceof InstanceConfigChangeListener) { + listenerClass = InstanceConfigChangeListener.class; +} +break; + case CLUSTER_CONFIG: +listenerClass = ClusterConfigChangeListener.class; +break; + case RESOURCE_CONFIG: +listenerClass = ResourceConfigChangeListener.class; +break; + case CUSTOMIZED_STATE_CONFIG: +listenerClass = CustomizedStateConfigChangeListener.class; +break; + case CONFIG: listenerClass = ConfigChangeListener.class; - } else if (_listener instanceof InstanceConfigChangeListener) { -listenerClass = InstanceConfigChangeListener.class; - } - break; -case CLUSTER_CONFIG: - listenerClass = ClusterConfigChangeListener.class; - break; -case RESOURCE_CONFIG: - listenerClass = ResourceConfigChangeListener.class; - break; -case CONFIG: - listenerClass = ConfigChangeListener.class; - break; -case LIVE_INSTANCE: - listenerClass = LiveInstanceChangeListener.class; - break; -case CURRENT_STATE: - listenerClass = CurrentStateChangeListener.class; - ; - break; -case MESSAGE: -case MESSAGES_CONTROLLER: - listenerClass = MessageListener.class; - break; -case EXTERNAL_VIEW: -case TARGET_EXTERNAL_VIEW: - listenerClass = ExternalViewChangeListener.class; - break; -case CUSTOMIZED_VIEW: - listenerClass = CustomizedViewChangeListener.class; - break; -case CONTROLLER: - listenerClass = ControllerChangeListener.class; +break; + case LIVE_INSTANCE: +listenerClass = LiveInstanceChangeListener.class; +break; + case CURRENT_STATE: +listenerClass = CurrentStateChangeListener.class; +break; + case CUSTOMIZED_STATE_ROOT: +listenerClass = CustomizedStateRootChangeListener.class; Review comment: I would prefer to keep them separate for clarity, and later when we get rid of the middle layer, we can delete the redundant one. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly closed issue #891: Test newly instrumented Helix Java APIs
narendly closed issue #891: Test newly instrumented Helix Java APIs URL: https://github.com/apache/helix/issues/891 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly opened a new issue #907: Make Helix REST realm-aware
narendly opened a new issue #907: Make Helix REST realm-aware URL: https://github.com/apache/helix/issues/907 Helix REST needs to start using a realm-aware ZkClient on multi-zk mode. Also it needs to become a listener on routing data because we don't want to restart the HelixRestServer every time we update the routing data. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] narendly opened a new pull request #908: Make Helix REST realm-aware
narendly opened a new pull request #908: Make Helix REST realm-aware URL: https://github.com/apache/helix/pull/908 ### Issues - [x] My PR addresses the following Helix issues and references them in the PR description: Fixes #907 ### Description - [x] Here are some details about my PR, including screenshots of any UI changes: Helix REST needs to start using a realm-aware ZkClient on multi-zk mode. Also it needs to become a listener on routing data because we don't want to restart the HelixRestServer every time we update the routing data. Changelist: 1. Make ServerContext listen on routing data paths if run on multi-zk mode 2. Make HelixRestServer use RealmAwareZkClient (FederatedZkClient) on multi-zk mode ### Tests - [x] The following tests are written for this issue: TestRoutingDataUpdate - [x] The following is the result of the "mvn test" command on the appropriate module: on helix-rest module: > [INFO] Results: > [INFO] > [ERROR] Failures: > [ERROR] TestResourceAccessor.testResourceHealth:289 expected: but was: > [INFO] > [ERROR] Tests run: 144, Failures: 1, Errors: 0, Skipped: 7 > [INFO] > [INFO] > [INFO] BUILD FAILURE > [INFO] > [INFO] Total time: 37.847 s > [INFO] Finished at: 2020-03-23T22:02:47-07:00 > [INFO] TestResourceAccessor is an unrelated failure that will be fixed in future iterations. ### Commits - [x] 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 - [x] 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org