[GitHub] [helix] alirezazamani commented on a change in pull request #355: Change IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm.

2019-07-23 Thread GitBox
alirezazamani commented on a change in pull request #355: Change IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm. URL: https://github.com/apache/helix/pull/355#discussion_r306518338 ## File path:

[GitHub] [helix] pkuwm edited a comment on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
pkuwm edited a comment on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514336391 I personally don't like null, either, since I see it as anti-pattern. Why I put null in this draft was I saw `return null` as well

[GitHub] [helix] pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514351120 Did you mean optional causes performance overhead? Just curious, can you explain as I don’t see it a performance overhead here? Thanks.

[GitHub] [helix] pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514336391 I personally don't like null, either, since I see it as anti-pattern. Why I put null in this draft was I saw `return null` as well in the

[GitHub] [helix] narendly commented on issue #355: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm.

2019-07-23 Thread GitBox
narendly commented on issue #355: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm. URL: https://github.com/apache/helix/pull/355#issuecomment-514328978 Could we reformat the PR according to the guidelines? Thanks :) -hunter On Jul 23,

[GitHub] [helix] narendly commented on issue #356: Read Failure while reading non-existent znode.

2019-07-23 Thread GitBox
narendly commented on issue #356: Read Failure while reading non-existent znode. URL: https://github.com/apache/helix/pull/356#issuecomment-514328130 Could you reformat the pull request according to the guidelines? Thanks! - Hunter On Jul 23, 2019, 20:19, at 20:19, Ali Reza Zamani

[GitHub] [helix] alirezazamani commented on a change in pull request #356: Read Failure while reading non-existent znode.

2019-07-23 Thread GitBox
alirezazamani commented on a change in pull request #356: Read Failure while reading non-existent znode. URL: https://github.com/apache/helix/pull/356#discussion_r306464047 ## File path: helix-core/src/main/java/org/apache/helix/manager/zk/zookeeper/ZkClient.java ## @@

[GitHub] [helix] narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514293990 I think Optional would be beneficial if you want to have fluid chained-calls (similar to functional programming). Here, it is possible

[GitHub] [helix] pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514291290 For the isWFConfig part, I understand. Definitely doable. Just for a discussion about Optional. I think it is designed to solve the

[GitHub] [helix] alirezazamani opened a new issue #358: removing DEFAULT_VIEW_CLUSTER_REFRESH_PERIOD from clusterconfig

2019-07-23 Thread GitBox
alirezazamani opened a new issue #358: removing DEFAULT_VIEW_CLUSTER_REFRESH_PERIOD from clusterconfig URL: https://github.com/apache/helix/issues/358 This is an automated message from the Apache Git Service. To respond to

[GitHub] [helix] narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514134881 As for the timeout value of 0, the semantics you proposed are a bit confusing. Perhaps enforce that the timeout value must be

[GitHub] [helix] pkuwm edited a comment on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
pkuwm edited a comment on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514130185 > As for parseWorkflow - I think it will be cleaner if we could have something like isWorkflowConfig() instead that returns T/F,

[GitHub] [helix] pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514130185 > As for parseWorkflow - I think it will be cleaner if we could have something like isWorkflowConfig() instead that returns T/F, and

[GitHub] [helix] pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514109056 Definitely could. I thought about it. getWorkflows() calls getWorkflows(0). But considering future.get can also accept 0 as timeouts.

[GitHub] [helix] narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514106795 And once you've done that, in your unit test, create a mock of getWorkflow () function, and make it take a long time (to simulate ZK

[GitHub] [helix] pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514106193 Right. I was actually considering that. But it seems after checking isWFConfig, parsing is still needed. To me it seems redundant.

[GitHub] [helix] narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514105616 Also, to follow the DRY principle - could you try to reuse the logic we have in the existing getWorkflows ()? getWorkflows(long

[GitHub] [helix] narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514104602 As for parseWorkflow - I think it will be cleaner if we could have something like isWorkflowConfig() instead that returns T/F, and

[GitHub] [helix] narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514101832 Please review the pull request guidelines and format your pull request accordingly. On Jul 23, 2019, 10:01, at 10:01, pkuwm

[GitHub] [helix] pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
pkuwm commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514099797 Wonder what the unit test requirement is. Can you please suggest? This

[GitHub] [helix] pkuwm opened a new pull request #357: [WIP] Add getWorkflows(long timeout) to TaskDriver.

2019-07-23 Thread GitBox
pkuwm opened a new pull request #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357 Sometimes zookeeper hangs in the getWorkflows() call, and we could provide an API with a timeout to handle this at the application level. Use

[GitHub] [helix] pkuwm commented on a change in pull request #356: Read Failure while reading non-existent znode.

2019-07-23 Thread GitBox
pkuwm commented on a change in pull request #356: Read Failure while reading non-existent znode. URL: https://github.com/apache/helix/pull/356#discussion_r306149611 ## File path: helix-core/src/main/java/org/apache/helix/manager/zk/zookeeper/ZkClient.java ## @@ -721,6

[GitHub] [helix] pkuwm commented on a change in pull request #355: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm.

2019-07-23 Thread GitBox
pkuwm commented on a change in pull request #355: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm. URL: https://github.com/apache/helix/pull/355#discussion_r306147335 ## File path:

[GitHub] [helix] alirezazamani opened a new pull request #356: Read Failure while reading non-existent znode.

2019-07-22 Thread GitBox
alirezazamani opened a new pull request #356: Read Failure while reading non-existent znode. URL: https://github.com/apache/helix/pull/356 In case of encountering NoNodeException while reading data from a znode that does not exist, readfailurecounter will be incremented which shouldn't

[GitHub] [helix] dasahcc commented on a change in pull request #355: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm.

2019-07-22 Thread GitBox
dasahcc commented on a change in pull request #355: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm. URL: https://github.com/apache/helix/pull/355#discussion_r306135786 ## File path:

[GitHub] [helix] jiajunwang commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer.

2019-07-22 Thread GitBox
jiajunwang commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer. URL: https://github.com/apache/helix/pull/348#discussion_r306132886 ## File path: helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java ## @@ -19,19

[GitHub] [helix] kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r306097821 ## File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestClusterAccessor.java

[GitHub] [helix] kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r306097771 ## File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestClusterAccessor.java

[GitHub] [helix] kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r306097649 ## File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestClusterAccessor.java

[GitHub] [helix] pkuwm commented on issue #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
pkuwm commented on issue #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#issuecomment-514017940 Sounds good! On Mon, Jul 22, 2019 at 6:34 PM kaisun2000 wrote: > *@kaisun2000* commented on this pull request. >

[GitHub] [helix] kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r306097170 ## File path:

[GitHub] [helix] pkuwm edited a comment on issue #349: Need to improve the code style & formatting regulation

2019-07-22 Thread GitBox
pkuwm edited a comment on issue #349: Need to improve the code style & formatting regulation URL: https://github.com/apache/helix/issues/349#issuecomment-514013728 How about the order of imports, to make the imports consistent in the project? Like: ``` import java.* import

[GitHub] [helix] pkuwm commented on issue #349: Need to improve the code style & formatting regulation

2019-07-22 Thread GitBox
pkuwm commented on issue #349: Need to improve the code style & formatting regulation URL: https://github.com/apache/helix/issues/349#issuecomment-514013728 How about the order of imports, to make the imports consistent in the project? Like: :::java import java.* import javax.*

[GitHub] [helix] pkuwm closed issue #353: Don't ignore exceptions: adding log messages to help check exceptions.

2019-07-22 Thread GitBox
pkuwm closed issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353 This is an automated message from the Apache Git Service. To respond to the

[GitHub] [helix] pkuwm commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions.

2019-07-22 Thread GitBox
pkuwm commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353#issuecomment-513992696 Sure. Agree on this: a problem could have multiple solutions. Maybe this is just a personal coding style. I just

[GitHub] [helix] narendly commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions.

2019-07-22 Thread GitBox
narendly commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353#issuecomment-513991286 Just to reiterate, since this part of the code is working correctly, I would just leave this as is. If you would

[GitHub] [helix] pkuwm commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions.

2019-07-22 Thread GitBox
pkuwm commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353#issuecomment-513991073 I agree with you. Instead of using an exception like way, maybe an if condition could solve the problem and avoid

[GitHub] [helix] narendly commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions.

2019-07-22 Thread GitBox
narendly commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353#issuecomment-513990562 Yes. Like I implied in my previous comment, there are certain fields that only show up in valid WorkflowConfigs.

[GitHub] [helix] pkuwm commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions.

2019-07-22 Thread GitBox
pkuwm commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353#issuecomment-513989903 Yes, it makes sense not to expose the config details. But just curious, regarding this workflowConfig

[GitHub] [helix] narendly commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions.

2019-07-22 Thread GitBox
narendly commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353#issuecomment-513988811 What you are saying makes sense. But for this exception specifically,  we do not want to log it because it will

[GitHub] [helix] jiajunwang merged pull request #341: Initial checking for the Weight-Aware Globally-Even Distribute rebalancer

2019-07-22 Thread GitBox
jiajunwang merged pull request #341: Initial checking for the Weight-Aware Globally-Even Distribute rebalancer URL: https://github.com/apache/helix/pull/341 This is an automated message from the Apache Git Service. To

[GitHub] [helix] jiajunwang commented on issue #341: Initial checking for the Weight-Aware Globally-Even Distribute rebalancer

2019-07-22 Thread GitBox
jiajunwang commented on issue #341: Initial checking for the Weight-Aware Globally-Even Distribute rebalancer URL: https://github.com/apache/helix/pull/341#issuecomment-513988590 As discussed with other team members, this check-in looks good in general. We might need to update it further

[GitHub] [helix] pkuwm commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions.

2019-07-22 Thread GitBox
pkuwm commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353#issuecomment-513987724 Thanks for the explanation! I understand what the code is doing. If the property is not a correct type, it continues

[GitHub] [helix] jiajunwang commented on issue #331: CallbackHandler async re-subscribe watcher can potentially miss events

2019-07-22 Thread GitBox
jiajunwang commented on issue #331: CallbackHandler async re-subscribe watcher can potentially miss events URL: https://github.com/apache/helix/issues/331#issuecomment-513984628 The root cause of the issue is a race condition in Helix cache refresh logic. Basically, Helix relies on the ZK

[GitHub] [helix] dasahcc commented on issue #354: No stats fetched for ResourceConfig through ConfigAccessor

2019-07-22 Thread GitBox
dasahcc commented on issue #354: No stats fetched for ResourceConfig through ConfigAccessor URL: https://github.com/apache/helix/issues/354#issuecomment-513981654 @narendly This is a bug since all the ZNode objects extends from HelixProperty which contains Stats. We should not have

[GitHub] [helix] narendly commented on issue #354: No stats fetched for ResourceConfig through ConfigAccessor

2019-07-22 Thread GitBox
narendly commented on issue #354: No stats fetched for ResourceConfig through ConfigAccessor URL: https://github.com/apache/helix/issues/354#issuecomment-513980123 Could we add a little more context for this? What do we need stats for? Can't we use DataAccessor for Stats instead?

[GitHub] [helix] dasahcc closed pull request #58: helix-core: AutoRebalancer should include only numbered states in `currentMapping`

2019-07-22 Thread GitBox
dasahcc closed pull request #58: helix-core: AutoRebalancer should include only numbered states in `currentMapping` URL: https://github.com/apache/helix/pull/58 This is an automated message from the Apache Git Service. To

[GitHub] [helix] dasahcc closed pull request #294: Implement view cluster aggregator

2019-07-22 Thread GitBox
dasahcc closed pull request #294: Implement view cluster aggregator URL: https://github.com/apache/helix/pull/294 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

[GitHub] [helix] dasahcc closed pull request #59: helix-core: AutoRebalanceStrategy should sort by partition count and state priority

2019-07-22 Thread GitBox
dasahcc closed pull request #59: helix-core: AutoRebalanceStrategy should sort by partition count and state priority URL: https://github.com/apache/helix/pull/59 This is an automated message from the Apache Git Service. To

[GitHub] [helix] dasahcc closed pull request #300: Daemonize SubscribeChangeEventProcessor thread in CallbackProcessor

2019-07-22 Thread GitBox
dasahcc closed pull request #300: Daemonize SubscribeChangeEventProcessor thread in CallbackProcessor URL: https://github.com/apache/helix/pull/300 This is an automated message from the Apache Git Service. To respond to the

[GitHub] [helix] dasahcc closed pull request #201: add null check in DeplayedAutoRebalancerz#computeNewIdealState

2019-07-22 Thread GitBox
dasahcc closed pull request #201: add null check in DeplayedAutoRebalancerz#computeNewIdealState URL: https://github.com/apache/helix/pull/201 This is an automated message from the Apache Git Service. To respond to the

[GitHub] [helix] dasahcc closed pull request #299: Remove unused _forceRebalanceTimer from GenericHelixController

2019-07-22 Thread GitBox
dasahcc closed pull request #299: Remove unused _forceRebalanceTimer from GenericHelixController URL: https://github.com/apache/helix/pull/299 This is an automated message from the Apache Git Service. To respond to the

[GitHub] [helix] dasahcc closed pull request #319: Fix long version of quickstart guide of helix 0.8.4 release.

2019-07-22 Thread GitBox
dasahcc closed pull request #319: Fix long version of quickstart guide of helix 0.8.4 release. URL: https://github.com/apache/helix/pull/319 This is an automated message from the Apache Git Service. To respond to the

[GitHub] [helix] dasahcc closed pull request #266: Propose design for aggregated cluster view service

2019-07-22 Thread GitBox
dasahcc closed pull request #266: Propose design for aggregated cluster view service URL: https://github.com/apache/helix/pull/266 This is an automated message from the Apache Git Service. To respond to the message, please

[GitHub] [helix] dasahcc closed pull request #200: throw new Exception to avoid ugly NPE

2019-07-22 Thread GitBox
dasahcc closed pull request #200: throw new Exception to avoid ugly NPE URL: https://github.com/apache/helix/pull/200 This is an automated message from the Apache Git Service. To respond to the message, please log on to

[GitHub] [helix] dasahcc closed pull request #26: [HELIX-592] addCluster should respect overwriteExisitng when adding stat...

2019-07-22 Thread GitBox
dasahcc closed pull request #26: [HELIX-592] addCluster should respect overwriteExisitng when adding stat... URL: https://github.com/apache/helix/pull/26 This is an automated message from the Apache Git Service. To respond

[GitHub] [helix] dasahcc closed pull request #1: [HELIX-60] CORS Enabled Dashboard for Apache Helix

2019-07-22 Thread GitBox
dasahcc closed pull request #1: [HELIX-60] CORS Enabled Dashboard for Apache Helix URL: https://github.com/apache/helix/pull/1 This is an automated message from the Apache Git Service. To respond to the message, please log

[GitHub] [helix] dasahcc opened a new issue #354: No stats fetched for ResourceConfig through ConfigAccessor

2019-07-22 Thread GitBox
dasahcc opened a new issue #354: No stats fetched for ResourceConfig through ConfigAccessor URL: https://github.com/apache/helix/issues/354 If you read ResourceConfig through ConfigAccessor, the stats are not pulled from ZK. Thus the stats data is incorrect with these objects. We

[GitHub] [helix] narendly commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions.

2019-07-22 Thread GitBox
narendly commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353#issuecomment-513972958 Actually, we do not want to log anything here. 1. This exception does not actually signal that anything is

[GitHub] [helix] narendly commented on issue #352: Don't log and throw: removing redundant logs right before throwing exception

2019-07-22 Thread GitBox
narendly commented on issue #352: Don't log and throw: removing redundant logs right before throwing exception URL: https://github.com/apache/helix/issues/352#issuecomment-513970542 It makes sense not to double-log things in the log and the exception. If we want to make this change,

[GitHub] [helix] pkuwm commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
pkuwm commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r306050637 ## File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java

[GitHub] [helix] kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r306041352 ## File path:

[GitHub] [helix] pkuwm commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
pkuwm commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r306039336 ## File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestClusterAccessor.java ##

[GitHub] [helix] pkuwm commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
pkuwm commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r306039336 ## File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestClusterAccessor.java ##

[GitHub] [helix] kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r306036658 ## File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestClusterAccessor.java

[GitHub] [helix] kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-22 Thread GitBox
kaisun2000 commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r306034091 ## File path:

[GitHub] [helix] pkuwm opened a new issue #353: Don't ignore exceptions: adding log messages to help check exceptions.

2019-07-22 Thread GitBox
pkuwm opened a new issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353 Ex.:

[GitHub] [helix] pkuwm opened a new issue #352: Don't log and throw: removing redundant logs right before throwing exception

2019-07-22 Thread GitBox
pkuwm opened a new issue #352: Don't log and throw: removing redundant logs right before throwing exception URL: https://github.com/apache/helix/issues/352 Same log message should not be logged right while an exception is also thrown. Ex.:

[GitHub] [helix] alirezazamani commented on a change in pull request #344: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm.

2019-07-22 Thread GitBox
alirezazamani commented on a change in pull request #344: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm. URL: https://github.com/apache/helix/pull/344#discussion_r305914218 ## File path:

[GitHub] [helix] alirezazamani closed pull request #346: Unwanted logs have been removed.

2019-07-22 Thread GitBox
alirezazamani closed pull request #346: Unwanted logs have been removed. URL: https://github.com/apache/helix/pull/346 This is an automated message from the Apache Git Service. To respond to the message, please log on to

[GitHub] [helix] alirezazamani closed pull request #344: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm.

2019-07-22 Thread GitBox
alirezazamani closed pull request #344: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm. URL: https://github.com/apache/helix/pull/344 This is an automated message from the

[GitHub] [helix] alirezazamani commented on a change in pull request #346: Unwanted logs have been removed.

2019-07-22 Thread GitBox
alirezazamani commented on a change in pull request #346: Unwanted logs have been removed. URL: https://github.com/apache/helix/pull/346#discussion_r305927600 ## File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/strategy/CrushRebalanceStrategy.java

[GitHub] [helix] alirezazamani opened a new issue #351: removing the todo log

2019-07-22 Thread GitBox
alirezazamani opened a new issue #351: removing the todo log URL: https://github.com/apache/helix/issues/351 If debugging NPE is complete, some if the logs can be removed for CRUSH based algorithm (in computeResourceBestPossibleState function).

[GitHub] [helix] alirezazamani commented on a change in pull request #344: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm.

2019-07-22 Thread GitBox
alirezazamani commented on a change in pull request #344: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm. URL: https://github.com/apache/helix/pull/344#discussion_r305914218 ## File path:

[GitHub] [helix] pkuwm opened a new issue #350: linkedlist

2019-07-21 Thread GitBox
pkuwm opened a new issue #350: linkedlist URL: https://github.com/apache/helix/issues/350 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

[GitHub] [helix] pkuwm commented on issue #326: Add getWorkflow(long timeout) to TaskDriver

2019-07-21 Thread GitBox
pkuwm commented on issue #326: Add getWorkflow(long timeout) to TaskDriver URL: https://github.com/apache/helix/issues/326#issuecomment-513576653 Awesome. I'll take it then. Thanks. This is an automated message from the

[GitHub] [helix] narendly commented on issue #326: Add getWorkflow(long timeout) to TaskDriver

2019-07-21 Thread GitBox
narendly commented on issue #326: Add getWorkflow(long timeout) to TaskDriver URL: https://github.com/apache/helix/issues/326#issuecomment-513569098 Feel free to contribute. I have not started working on it yet. The implementation should be straightforward and I could help with the review.

[GitHub] [helix] pkuwm commented on issue #326: Add getWorkflow(long timeout) to TaskDriver

2019-07-21 Thread GitBox
pkuwm commented on issue #326: Add getWorkflow(long timeout) to TaskDriver URL: https://github.com/apache/helix/issues/326#issuecomment-513568933 Thanks. Are you working on it? If not, may I take this ticket and contribute? On Sun, Jul 21, 2019 at 1:33 AM Hunter Lee wrote:

[GitHub] [helix] narendly commented on issue #349: Need to improve the code style & formatting regulation

2019-07-21 Thread GitBox
narendly commented on issue #349: Need to improve the code style & formatting regulation URL: https://github.com/apache/helix/issues/349#issuecomment-513535129 +1 If possible, let us also add the Apache license at the top of every file. Hunter On Sat, Jul 20, 2019,

[GitHub] [helix] narendly commented on issue #326: Add getWorkflow(long timeout) to TaskDriver

2019-07-21 Thread GitBox
narendly commented on issue #326: Add getWorkflow(long timeout) to TaskDriver URL: https://github.com/apache/helix/issues/326#issuecomment-513534644 It is a typo - it is getWorkflows(). For now, only adding getWorkflows (timeout) would suffice. Hunter ⁣Sent from BlueMail ​

[GitHub] [helix] pkuwm commented on issue #326: Add getWorkflow(long timeout) to TaskDriver

2019-07-21 Thread GitBox
pkuwm commented on issue #326: Add getWorkflow(long timeout) to TaskDriver URL: https://github.com/apache/helix/issues/326#issuecomment-513532438 I don't see the call: getWorkflow() in the source code. Did you mean

[GitHub] [helix] pkuwm commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer.

2019-07-21 Thread GitBox
pkuwm commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer. URL: https://github.com/apache/helix/pull/348#discussion_r305572150 ## File path: helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java ## @@ -19,19

[GitHub] [helix] jiajunwang opened a new issue #349: Need to improve the code style & formatting regulation

2019-07-20 Thread GitBox
jiajunwang opened a new issue #349: Need to improve the code style & formatting regulation URL: https://github.com/apache/helix/issues/349 The current format file helix-style.xml has very limited configured items. This leaves too much flexibility to Helix developers. We shall add more

[GitHub] [helix] pkuwm commented on a change in pull request #333: Fix issue when client only sets ANY at cluster level throttle config

2019-07-20 Thread GitBox
pkuwm commented on a change in pull request #333: Fix issue when client only sets ANY at cluster level throttle config URL: https://github.com/apache/helix/pull/333#discussion_r305588139 ## File path:

[GitHub] [helix] pkuwm commented on a change in pull request #344: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm.

2019-07-20 Thread GitBox
pkuwm commented on a change in pull request #344: Changing IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm. URL: https://github.com/apache/helix/pull/344#discussion_r305587778 ## File path:

[GitHub] [helix] pkuwm commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-20 Thread GitBox
pkuwm commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r305587698 ## File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java

[GitHub] [helix] dasahcc commented on a change in pull request #346: Unwanted logs have been removed.

2019-07-20 Thread GitBox
dasahcc commented on a change in pull request #346: Unwanted logs have been removed. URL: https://github.com/apache/helix/pull/346#discussion_r305574275 ## File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/strategy/CrushRebalanceStrategy.java ##

[GitHub] [helix] dasahcc commented on a change in pull request #346: Unwanted logs have been removed.

2019-07-20 Thread GitBox
dasahcc commented on a change in pull request #346: Unwanted logs have been removed. URL: https://github.com/apache/helix/pull/346#discussion_r305574275 ## File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/strategy/CrushRebalanceStrategy.java ##

[GitHub] [helix] dasahcc commented on a change in pull request #346: Unwanted logs have been removed.

2019-07-20 Thread GitBox
dasahcc commented on a change in pull request #346: Unwanted logs have been removed. URL: https://github.com/apache/helix/pull/346#discussion_r305574112 ## File path: helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java ## @@

[GitHub] [helix] pkuwm commented on a change in pull request #346: Unwanted logs have been removed.

2019-07-20 Thread GitBox
pkuwm commented on a change in pull request #346: Unwanted logs have been removed. URL: https://github.com/apache/helix/pull/346#discussion_r305573518 ## File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/strategy/CrushRebalanceStrategy.java ## @@

[GitHub] [helix] pkuwm commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer.

2019-07-20 Thread GitBox
pkuwm commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer. URL: https://github.com/apache/helix/pull/348#discussion_r305572150 ## File path: helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java ## @@ -19,19

[GitHub] [helix] alirezazamani commented on a change in pull request #346: Unwanted logs have been removed.

2019-07-20 Thread GitBox
alirezazamani commented on a change in pull request #346: Unwanted logs have been removed. URL: https://github.com/apache/helix/pull/346#discussion_r305572008 ## File path: helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java ## @@

[GitHub] [helix] jiajunwang commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer.

2019-07-20 Thread GitBox
jiajunwang commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer. URL: https://github.com/apache/helix/pull/348#discussion_r305571783 ## File path: helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java ## @@ -19,19

[GitHub] [helix] jiajunwang commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer.

2019-07-20 Thread GitBox
jiajunwang commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer. URL: https://github.com/apache/helix/pull/348#discussion_r305571617 ## File path: helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java ## @@

[GitHub] [helix] pkuwm commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer.

2019-07-20 Thread GitBox
pkuwm commented on a change in pull request #348: Adding the configuration items of the WAGED rebalancer. URL: https://github.com/apache/helix/pull/348#discussion_r305571503 ## File path: helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java ## @@ -505,6

[GitHub] [helix] jiajunwang opened a new pull request #348: Adding the configuration items of the WAGED rebalancer.

2019-07-20 Thread GitBox
jiajunwang opened a new pull request #348: Adding the configuration items of the WAGED rebalancer. URL: https://github.com/apache/helix/pull/348 ### Issues #342 ### Description This PR includes the necessary configuration items to support the WAGED rebalancer. ###

[GitHub] [helix] jiajunwang commented on issue #331: CallbackHandler async re-subscribe watcher can potentially miss events

2019-07-19 Thread GitBox
jiajunwang commented on issue #331: CallbackHandler async re-subscribe watcher can potentially miss events URL: https://github.com/apache/helix/issues/331#issuecomment-513437251 The re-subscribe does not rely on the async logic. It is by default done in the ZkClient. The real problem

[GitHub] [helix] lei-xia commented on a change in pull request #346: Unwanted logs have been removed.

2019-07-19 Thread GitBox
lei-xia commented on a change in pull request #346: Unwanted logs have been removed. URL: https://github.com/apache/helix/pull/346#discussion_r305564620 ## File path: helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java ## @@

[GitHub] [helix] lei-xia commented on issue #346: Unwanted logs have been removed.

2019-07-19 Thread GitBox
lei-xia commented on issue #346: Unwanted logs have been removed. URL: https://github.com/apache/helix/pull/346#issuecomment-513428194 Is this fixing a single issue? Why there are 4 commits? Can they be combined to one commit?

[GitHub] [helix] pkuwm commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0

2019-07-19 Thread GitBox
pkuwm commented on a change in pull request #339: Implementation of stateModelDef modification in REST 2.0 URL: https://github.com/apache/helix/pull/339#discussion_r305563733 ## File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java

<    1   2   3   4   5   6   7   >