[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/helix/pull/145


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r176489075
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/AbstractRebalanceSoftConstraint.java
 ---
@@ -0,0 +1,56 @@
+package org.apache.helix.api.rebalancer.constraint;
+
+/*
+ * 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.controller.common.ResourcesStateMap;
+
+import java.util.Map;
+
+public abstract class AbstractRebalanceSoftConstraint {
+  private static int DEFAULT_IMPORTANCE = 1;
+  protected int _importance = DEFAULT_IMPORTANCE;
+
+  /**
+   * Evaluate how the given assignment fits the constraint.
+   * @param resource Target resource
+   * @param proposedAssignment Map of 
+   * @return Evaluation about the assignment. Larger number means better 
fit under this constraint.
+   */
+  public abstract Map evaluate(String resource,
+  Map proposedAssignment);
+
+  /**
+   * @return The soft constraint's importance that will be used to compare 
with other soft constraint results.
+   * Aggregated evaluation score = SUM(constraint_evaluation * importance).
+   */
+  public int getConstraintImportance() {
--- End diff --

Good to add that larger means more important. I am still not fully 
convinced the importance is the right name for this. Maybe @kishoreg  has other 
ideas?  I mentioned importance in a comment because I got confused between the 
usage of the word weight in various places.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r176490613
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/CapacityProvider.java
 ---
@@ -0,0 +1,48 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting participant capacity information.
+ * The return value will be used to estimate available capacity, as well 
as utilization in percentage for prioritizing participants.
+ *
+ * Note that all return values of the provider are supposed to be in the 
same unit.
+ * For example, if the provider is for memory capacity of a host (total 
memory size is 8G, current usage is 512MB),
+ * the return values could be {@link 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider#getParticipantCapacity(String)}
 = 8192, and {@link 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider#getParticipantUsage(String)}
 = 512.
+ * Another example, if the provider is for partition count capacity (max 
count 1000 partitions, currently no partition assigned),
+ * the return values should be {@link 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider#getParticipantCapacity(String)}
 = 1000, and {@link 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider#getParticipantUsage(String)}
 = 0.
+ *
+ * Moreover, while this provider is used together with a {@link 
org.apache.helix.api.rebalancer.constraint.dataprovider.PartitionWeightProvider 
PartitionWeightProvider},
+ * both providers are supposed to return values in the same unit so they 
can be used to estimate the resource usage of an proposed assignment.
+ */
+public interface CapacityProvider {
+
+  /**
+   * @param participant
+   * @return The total participant capacity.
+   */
+  int getParticipantCapacity(String participant);
+
+  /**
+   * @param participant
+   * @return The participant usage (provisioned capacity).
--- End diff --

The comment "(provisioned capacity)" is confusing. Can we remove it?


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r176488707
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/AbstractRebalanceSoftConstraint.java
 ---
@@ -0,0 +1,56 @@
+package org.apache.helix.api.rebalancer.constraint;
+
+/*
+ * 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.controller.common.ResourcesStateMap;
+
+import java.util.Map;
+
+public abstract class AbstractRebalanceSoftConstraint {
+  private static int DEFAULT_IMPORTANCE = 1;
+  protected int _importance = DEFAULT_IMPORTANCE;
+
+  /**
+   * Evaluate how the given assignment fits the constraint.
+   * @param resource Target resource
+   * @param proposedAssignment Map of 
+   * @return Evaluation about the assignment. Larger number means better 
fit under this constraint.
--- End diff --

@return score of the assignment?


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r175879486
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/controller/rebalancer/constraint/TotalCapacityConstraint.java
 ---
@@ -0,0 +1,79 @@
+package org.apache.helix.controller.rebalancer.constraint;
+
+/*
+ * 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.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.PartitionWeightProvider;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import org.apache.helix.controller.rebalancer.util.ResourceUsageCalculator;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class TotalCapacityConstraint extends 
AbstractRebalanceHardConstraint {
+  private final PartitionWeightProvider _partitionWeightProvider;
+  private final CapacityProvider _capacityProvider;
+  // Use to track any assignments that are proposed during the rebalance 
process.
+  // Note these assignments are not reflected in providers.
+  private final Map _pendingUsage;
+
+  public TotalCapacityConstraint(PartitionWeightProvider 
partitionWeightProvider,
+  CapacityProvider capacityProvider) {
+super();
+_partitionWeightProvider = partitionWeightProvider;
+_capacityProvider = capacityProvider;
+_pendingUsage = new HashMap<>();
+  }
+
+  private boolean validate(String resource, String partition, String 
participant) {
+int usage = _capacityProvider.getParticipantUsage(participant) + 
(_pendingUsage
+.containsKey(participant) ? _pendingUsage.get(participant) : 0);
+return +_partitionWeightProvider.getPartitionWeight(resource, 
partition) + usage
--- End diff --

Typo... Good catch.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r175879053
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/AbstractRebalanceSoftConstraint.java
 ---
@@ -0,0 +1,56 @@
+package org.apache.helix.api.rebalancer.constraint;
+
+/*
+ * 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.controller.common.ResourcesStateMap;
+
+import java.util.Map;
+
+public abstract class AbstractRebalanceSoftConstraint {
+  private static int DEFAULT_IMPORTANCE = 1;
+  protected int _importance = DEFAULT_IMPORTANCE;
+
+  /**
+   * Evaluate how the given assignment fits the constraint.
+   * @param resource Target resource
+   * @param proposedAssignment Map of 
+   * @return Evaluation about the assignment. Larger number means better 
fit under this constraint.
+   */
+  public abstract Map evaluate(String resource,
+  Map proposedAssignment);
+
+  /**
+   * @return The soft constraint's importance that will be used to compare 
with other soft constraint results.
+   * Aggregated evaluation score = SUM(constraint_evaluation * importance).
+   */
+  public int getConstraintImportance() {
--- End diff --

It's basically weight of the constraint. However, we already have a weight 
for resource quota.
So avoid using the same word here.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[lei-xia]

Github user lei-xia commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r175858026
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/controller/rebalancer/constraint/TotalCapacityConstraint.java
 ---
@@ -0,0 +1,79 @@
+package org.apache.helix.controller.rebalancer.constraint;
+
+/*
+ * 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.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.PartitionWeightProvider;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import org.apache.helix.controller.rebalancer.util.ResourceUsageCalculator;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class TotalCapacityConstraint extends 
AbstractRebalanceHardConstraint {
+  private final PartitionWeightProvider _partitionWeightProvider;
+  private final CapacityProvider _capacityProvider;
+  // Use to track any assignments that are proposed during the rebalance 
process.
+  // Note these assignments are not reflected in providers.
+  private final Map _pendingUsage;
+
+  public TotalCapacityConstraint(PartitionWeightProvider 
partitionWeightProvider,
+  CapacityProvider capacityProvider) {
+super();
+_partitionWeightProvider = partitionWeightProvider;
+_capacityProvider = capacityProvider;
+_pendingUsage = new HashMap<>();
+  }
+
+  private boolean validate(String resource, String partition, String 
participant) {
+int usage = _capacityProvider.getParticipantUsage(participant) + 
(_pendingUsage
+.containsKey(participant) ? _pendingUsage.get(participant) : 0);
+return +_partitionWeightProvider.getPartitionWeight(resource, 
partition) + usage
--- End diff --

why there is a "+" in begining of the statement?


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[lei-xia]

Github user lei-xia commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r175833400
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/AbstractRebalanceSoftConstraint.java
 ---
@@ -0,0 +1,56 @@
+package org.apache.helix.api.rebalancer.constraint;
+
+/*
+ * 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.controller.common.ResourcesStateMap;
+
+import java.util.Map;
+
+public abstract class AbstractRebalanceSoftConstraint {
+  private static int DEFAULT_IMPORTANCE = 1;
+  protected int _importance = DEFAULT_IMPORTANCE;
+
+  /**
+   * Evaluate how the given assignment fits the constraint.
+   * @param resource Target resource
+   * @param proposedAssignment Map of 
+   * @return Evaluation about the assignment. Larger number means better 
fit under this constraint.
+   */
+  public abstract Map evaluate(String resource,
+  Map proposedAssignment);
+
+  /**
+   * @return The soft constraint's importance that will be used to compare 
with other soft constraint results.
+   * Aggregated evaluation score = SUM(constraint_evaluation * importance).
+   */
+  public int getConstraintImportance() {
--- End diff --

Is importance same as priority?


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[lei-xia]

Github user lei-xia commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r175833652
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/AbstractRebalanceSoftConstraint.java
 ---
@@ -0,0 +1,56 @@
+package org.apache.helix.api.rebalancer.constraint;
+
+/*
+ * 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.controller.common.ResourcesStateMap;
+
+import java.util.Map;
+
+public abstract class AbstractRebalanceSoftConstraint {
+  private static int DEFAULT_IMPORTANCE = 1;
+  protected int _importance = DEFAULT_IMPORTANCE;
+
+  /**
+   * Evaluate how the given assignment fits the constraint.
+   * @param resource Target resource
+   * @param proposedAssignment Map of 
+   * @return Evaluation about the assignment. Larger number means better 
fit under this constraint.
+   */
+  public abstract Map evaluate(String resource,
+  Map proposedAssignment);
+
+  /**
+   * @return The soft constraint's importance that will be used to compare 
with other soft constraint results.
+   * Aggregated evaluation score = SUM(constraint_evaluation * importance).
+   */
+  public int getConstraintImportance() {
--- End diff --

And the larger means more important?


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174576991
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/CapacityProvider.java
 ---
@@ -0,0 +1,38 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting participant capacity information.
+ */
+public interface CapacityProvider {
+
+  /**
+   * @param participant
+   * @return The total participant capacity.
+   */
+  int getParticipantCapacity(String participant);
+
+  /**
+   * @param participant
+   * @return The provisioned capacity.
+   */
+  int getParticipantProvisioned(String participant);
--- End diff --

Make sense. Let me add more comments.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174576953
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
+ * apply the change.
+ */
--- End diff --

I've already created the class, haha.

I will start with 3 examples, basic case, use ZK, and topology aware.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174576961
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
+ * apply the change.
+ */
+public class WeightAwareRebalanceUtil {
+  private final ClusterConfig _clusterConfig;
+  private final Map _instanceConfigMap = new 
HashMap<>();
+  // For the possible customized state models.
+  private final Map _stateModelDefs = new 
HashMap<>();
+  private final ClusterDataCache _dataCache;
+
+  public enum RebalanceOption {
+INIT,
+REASSIGN
+  }
+
+  public WeightAwareRebalanceUtil(ClusterConfig clusterConfig,
+  List instanceConfigs) {
+for (InstanceConfig instanceConfig : instanceConfigs) {
+  _instanceConfigMap.put(instanceConfig.getInstanceName(), 
instanceConfig);
+}
+_clusterConfig = clusterConfig;
+
+_dataCache = new ClusterDataCache();
+_dataCache.setInstanceConfigMap(_instanceConfigMap);
+_dataCache.setClusterConfig(_clusterConfig);
+List liveInstanceList = new ArrayList<>();
+for (String instance : _instanceConfigMap.keySet()) {
+  LiveInstance liveInstance = new LiveInstance(instance);
+  liveInstanceList.add(liveInstance);
+}
+_dataCache.setLiveInstances(liveInstanceList);
+  }
+
+  /**
+   * The method to generate partition assignment mappings.
+   *
+   * @param resourceConfigsConfig of all the resources that need to be 
rebalanced.
+   *   The tool throws Exception if any resource 
has no IS or broken/uninitialized IS.
+   *   The tool throws Exception if any resource 
is in full-auto mode.
+   * @param existingAssignment The existing partition assignment of the 
input resources.
+   * @param option INIT or REASSIGN
+   *   INIT: Keep existing assignment. Only 
generate new partition assignment.
--- End diff --

You are right, Subbu.
The existingAssignment is old partition assignment.
For incremental, those old assignments won't be changed unless they are not 
valid (compared with State Model requirement).
For full rebalance, the old assignments are only used to minimize movement 
if possible. But all existing assignment will be recalculated.

And totally agree with your suggestion that make 2 separate APIs. This also 
helps clean the javadoc for parameters.
I will do it. Thanks for the comment.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174526219
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/PartitionQuotaProvider.java
 ---
@@ -0,0 +1,33 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting partition quota information.
+ */
+public interface PartitionQuotaProvider {
+
+  /**
+   * @param resource
+   * @param partition
+   * @return The quota that is required by a certain partition in the 
specified resource
+   */
+  int getPartitionQuota(String resource, String partition);
--- End diff --

Clarify it with comments int he javadoc, with examples. I prefer weight 
rather than quota

A specific example class with a lot of javadocs on different scenarios and 
how these classes are to be used, will not hurt. Tests can serve as examples, 
but they are usually sloppy, and if they are examples, then they tend to be 
copy-pasted the same way in real code.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174525270
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/CapacityProvider.java
 ---
@@ -0,0 +1,38 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting participant capacity information.
+ */
+public interface CapacityProvider {
+
+  /**
+   * @param participant
+   * @return The total participant capacity.
+   */
+  int getParticipantCapacity(String participant);
+
+  /**
+   * @param participant
+   * @return The provisioned capacity.
+   */
+  int getParticipantProvisioned(String participant);
--- End diff --

Please provide examples in the javadoc. In this case, I would add something 
like: "For example if the constraint is on memory used by a resource in a 
participant, and the participant is provisioned with a max of 10MB for 
resources, out of which 40k is used, the getParticpantUsage() should return 40, 
and getParticipantCapacity() should return 10. On the other hand, if the 
constraint is on number of partitions in a participant, and the particpant has 
been provisioned to for a max of 1200 partitions, but has only 65, then 
getParticipantUsage() should return 65, and getParticipantCapacity() should 
return 1200" -- something like this. It does not hurt to have more comments. 
You will get less questions.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174527442
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
+ * apply the change.
+ */
--- End diff --

I cannot emphasize the importance of an example more. Like I mentioned 
elsewhere, I would suggest adding a WeightBasedRebalanceExample class, in which 
you can provide example of a few different scenarios (number of partitions as 
weight, partitin size as weight, a combination of these two, etc.). Add plenty 
of javadoc in the example class to demonstrate the usage.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174528962
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
+ * apply the change.
+ */
+public class WeightAwareRebalanceUtil {
+  private final ClusterConfig _clusterConfig;
+  private final Map _instanceConfigMap = new 
HashMap<>();
+  // For the possible customized state models.
+  private final Map _stateModelDefs = new 
HashMap<>();
+  private final ClusterDataCache _dataCache;
+
+  public enum RebalanceOption {
+INIT,
+REASSIGN
+  }
+
+  public WeightAwareRebalanceUtil(ClusterConfig clusterConfig,
+  List instanceConfigs) {
+for (InstanceConfig instanceConfig : instanceConfigs) {
+  _instanceConfigMap.put(instanceConfig.getInstanceName(), 
instanceConfig);
+}
+_clusterConfig = clusterConfig;
+
+_dataCache = new ClusterDataCache();
+_dataCache.setInstanceConfigMap(_instanceConfigMap);
+_dataCache.setClusterConfig(_clusterConfig);
+List liveInstanceList = new ArrayList<>();
+for (String instance : _instanceConfigMap.keySet()) {
+  LiveInstance liveInstance = new LiveInstance(instance);
+  liveInstanceList.add(liveInstance);
+}
+_dataCache.setLiveInstances(liveInstanceList);
+  }
+
+  /**
+   * The method to generate partition assignment mappings.
+   *
+   * @param resourceConfigsConfig of all the resources that need to be 
rebalanced.
+   *   The tool throws Exception if any resource 
has no IS or broken/uninitialized IS.
+   *   The tool throws Exception if any resource 
is in full-auto mode.
+   * @param existingAssignment The existing partition assignment of the 
input resources.
+   * @param option INIT or REASSIGN
+   *   INIT: Keep existing assignment. Only 
generate new partition assignment.
--- End diff --

If you are having difficulty naming the variable, use two different APIs.

buildIncrementalRebalanceAssignment()
buildFullRebalanceAssignment()

Or some such.

I still don't understand the difference between the two. 

I suppose incremental means to calculate idealstate assignment for new 
partitions, given the old partition assignment. Right? In that case the list of 
new partitions should be taken as an argument, but it is not. Old partitions 
are not touched. Am I right?

I suppose full rebalance means to re-assign existing partitions around (but 
no new partitions added). Am I right? In  this case, we rebalance the 
partitions, but minimize movement. Am I right?


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174326641
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
+ * apply the change.
+ */
+public class WeightAwareRebalanceUtil {
+  private final ClusterConfig _clusterConfig;
+  private final Map _instanceConfigMap = new 
HashMap<>();
+  // For the possible customized state models.
+  private final Map _stateModelDefs = new 
HashMap<>();
+  private final ClusterDataCache _dataCache;
+
+  public enum RebalanceOption {
+INIT,
+REASSIGN
+  }
+
+  public WeightAwareRebalanceUtil(ClusterConfig clusterConfig,
+  List instanceConfigs) {
+for (InstanceConfig instanceConfig : instanceConfigs) {
+  _instanceConfigMap.put(instanceConfig.getInstanceName(), 
instanceConfig);
+}
+_clusterConfig = clusterConfig;
+
+_dataCache = new ClusterDataCache();
+_dataCache.setInstanceConfigMap(_instanceConfigMap);
+_dataCache.setClusterConfig(_clusterConfig);
+List liveInstanceList = new ArrayList<>();
+for (String instance : _instanceConfigMap.keySet()) {
+  LiveInstance liveInstance = new LiveInstance(instance);
+  liveInstanceList.add(liveInstance);
+}
+_dataCache.setLiveInstances(liveInstanceList);
+  }
+
+  /**
+   * The method to generate partition assignment mappings.
+   *
+   * @param resourceConfigsConfig of all the resources that need to be 
rebalanced.
+   *   The tool throws Exception if any resource 
has no IS or broken/uninitialized IS.
+   *   The tool throws Exception if any resource 
is in full-auto mode.
+   * @param existingAssignment The existing partition assignment of the 
input resources.
+   * @param option INIT or REASSIGN
+   *   INIT: Keep existing assignment. Only 
generate new partition assignment.
--- End diff --

And about the last point. You are correct. Since the rebalance algorithm is 
optimized to minimize movement, even with full rebalance, the new mapping is 
supposed to keep as much same assignment as the old one. Just not guaranteed.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174326343
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
+ * apply the change.
+ */
+public class WeightAwareRebalanceUtil {
+  private final ClusterConfig _clusterConfig;
+  private final Map _instanceConfigMap = new 
HashMap<>();
+  // For the possible customized state models.
+  private final Map _stateModelDefs = new 
HashMap<>();
+  private final ClusterDataCache _dataCache;
+
+  public enum RebalanceOption {
+INIT,
+REASSIGN
+  }
+
+  public WeightAwareRebalanceUtil(ClusterConfig clusterConfig,
+  List instanceConfigs) {
+for (InstanceConfig instanceConfig : instanceConfigs) {
+  _instanceConfigMap.put(instanceConfig.getInstanceName(), 
instanceConfig);
+}
+_clusterConfig = clusterConfig;
+
+_dataCache = new ClusterDataCache();
+_dataCache.setInstanceConfigMap(_instanceConfigMap);
+_dataCache.setClusterConfig(_clusterConfig);
+List liveInstanceList = new ArrayList<>();
+for (String instance : _instanceConfigMap.keySet()) {
+  LiveInstance liveInstance = new LiveInstance(instance);
+  liveInstanceList.add(liveInstance);
+}
+_dataCache.setLiveInstances(liveInstanceList);
+  }
+
+  /**
+   * The method to generate partition assignment mappings.
+   *
+   * @param resourceConfigsConfig of all the resources that need to be 
rebalanced.
+   *   The tool throws Exception if any resource 
has no IS or broken/uninitialized IS.
+   *   The tool throws Exception if any resource 
is in full-auto mode.
+   * @param existingAssignment The existing partition assignment of the 
input resources.
+   * @param option INIT or REASSIGN
+   *   INIT: Keep existing assignment. Only 
generate new partition assignment.
--- End diff --

"Mode" is confusing while we already have "RebalanceMode".

About the option name, how about "incremental" rebalance & "full" 
rebalance? I think those 2 names are much better.

I will add the comment about the resourceConfig, this is where users should 
specify the partitions.
Also, as a standalone tool, most rebalance options won't work even 
configured in the cluster config / instance config. I will add comments to 
point it out.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174326382
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
+ * apply the change.
+ */
--- End diff --

Good suggestion. Will add it.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174324850
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
--- End diff --

Good point. Changed.
The tool is simplified version of rebalancer. We are planning a new 
rebalancer that calculates assignment according to constraints. But there will 
be much more detail logics for different rebalance requirements.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174324068
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/PartitionQuotaProvider.java
 ---
@@ -0,0 +1,33 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting partition quota information.
+ */
+public interface PartitionQuotaProvider {
--- End diff --

Should be PartitionWeightProvider based on previous comments and 
discussions.
Info would be too generic here, participant weight is strictly related to 
the participant capacity.

Given we have more date defined for the constraints, we can have more 
provider.
Constraint interface is not related to these providers. So we can always 
extend the provider list later.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174323268
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/controller/rebalancer/constraint/PartitionWeightAwareEvennessConstraint.java
 ---
@@ -0,0 +1,94 @@
+package org.apache.helix.controller.rebalancer.constraint;
+
+/*
+ * 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.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.PartitionQuotaProvider;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import org.apache.helix.controller.rebalancer.util.ResourceUsageCalculator;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class PartitionWeightAwareEvennessConstraint extends 
AbstractRebalanceSoftConstraint {
+  private final int _weight;
+  private final PartitionQuotaProvider _partitionQuotaProvider;
+  private final CapacityProvider _capacityProvider;
+  // Use to track any assignments that are proposed during the rebalance 
process.
+  // Note these assignments are not reflected in providers.
+  private final Map _pendingUsage;
+
+  public PartitionWeightAwareEvennessConstraint(PartitionQuotaProvider 
partitionQuotaProvider,
+  CapacityProvider capacityProvider, int weight) {
+_partitionQuotaProvider = partitionQuotaProvider;
+_capacityProvider = capacityProvider;
+_weight = weight;
--- End diff --

You are correct.

And Importance is a good name, I think. Let me change it and add some 
comments.
Also moved to the superclass.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174323126
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/CapacityProvider.java
 ---
@@ -0,0 +1,38 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting participant capacity information.
+ */
+public interface CapacityProvider {
+
+  /**
+   * @param participant
+   * @return The total participant capacity.
+   */
+  int getParticipantCapacity(String participant);
--- End diff --

This API for total capacity. Since we need to calculate the percentage of 
usage for prioritizing, we need both total capacity and current usage.
Given this, the second option looks better. Thanks for the comments. I will 
make the change.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174323213
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/PartitionQuotaProvider.java
 ---
@@ -0,0 +1,33 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting partition quota information.
+ */
+public interface PartitionQuotaProvider {
+
+  /**
+   * @param resource
+   * @param partition
+   * @return The quota that is required by a certain partition in the 
specified resource
+   */
+  int getPartitionQuota(String resource, String partition);
--- End diff --

It is.
I'm struggling to make the choice between weight or quota. I named it quota 
to avoid confusing with the "constraint weight".

Given we named the constraint weight to importance, let's name it to weight 
here.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

Github user jiajunwang commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174323138
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/CapacityProvider.java
 ---
@@ -0,0 +1,38 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting participant capacity information.
+ */
+public interface CapacityProvider {
+
+  /**
+   * @param participant
+   * @return The total participant capacity.
+   */
+  int getParticipantCapacity(String participant);
+
+  /**
+   * @param participant
+   * @return The provisioned capacity.
+   */
+  int getParticipantProvisioned(String participant);
--- End diff --

Let's use getParticipantUsage()


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174314122
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
+ * apply the change.
+ */
+public class WeightAwareRebalanceUtil {
+  private final ClusterConfig _clusterConfig;
+  private final Map _instanceConfigMap = new 
HashMap<>();
+  // For the possible customized state models.
+  private final Map _stateModelDefs = new 
HashMap<>();
+  private final ClusterDataCache _dataCache;
+
+  public enum RebalanceOption {
+INIT,
+REASSIGN
+  }
+
+  public WeightAwareRebalanceUtil(ClusterConfig clusterConfig,
+  List instanceConfigs) {
+for (InstanceConfig instanceConfig : instanceConfigs) {
+  _instanceConfigMap.put(instanceConfig.getInstanceName(), 
instanceConfig);
+}
+_clusterConfig = clusterConfig;
+
+_dataCache = new ClusterDataCache();
+_dataCache.setInstanceConfigMap(_instanceConfigMap);
+_dataCache.setClusterConfig(_clusterConfig);
+List liveInstanceList = new ArrayList<>();
+for (String instance : _instanceConfigMap.keySet()) {
+  LiveInstance liveInstance = new LiveInstance(instance);
+  liveInstanceList.add(liveInstance);
+}
+_dataCache.setLiveInstances(liveInstanceList);
+  }
+
+  /**
+   * The method to generate partition assignment mappings.
+   *
+   * @param resourceConfigsConfig of all the resources that need to be 
rebalanced.
+   *   The tool throws Exception if any resource 
has no IS or broken/uninitialized IS.
+   *   The tool throws Exception if any resource 
is in full-auto mode.
+   * @param existingAssignment The existing partition assignment of the 
input resources.
+   * @param option INIT or REASSIGN
+   *   INIT: Keep existing assignment. Only 
generate new partition assignment.
--- End diff --

instead of option, can we call it 'mode'? Also, it is not clear where we 
specify new partitions to be assigned. "INIT" also seems to be misleading. Why 
not "KEEP"? If "INIT" is specified, then is it true that the existing 
assignments will not be moved? if "REASSIGN" is specified, is it true that the 
algorithm will try not to move existing assignments,  but may do so?


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174310141
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/controller/rebalancer/constraint/dataprovider/ZkBasedCapacityProvider.java
 ---
@@ -0,0 +1,202 @@
+package org.apache.helix.controller.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.*;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
+import org.apache.helix.store.zk.ZkHelixPropertyStore;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A capacity provider based on ZK node.
+ * This class support persistent through Helix Property Store.
+ */
+public class ZkBasedCapacityProvider implements CapacityProvider {
+  public static final int DEFAULT_CAPACITY_VALUE = 0;
+  private static final String ROOT = "/PARTICIPANT_CAPACITY";
+
+  private final ZkHelixPropertyStore _propertyStore;
+  private final String _dimensionPath;
+  private ParticipantCapacity _capacity;
+
+  /**
+   * @param propertyStore The store that will be used to persist capacity 
information.
+   * @param dimensionName Identify of the capacity attribute. For example 
memory, CPU.
+   */
+  public ZkBasedCapacityProvider(ZkHelixPropertyStore 
propertyStore,
+  String dimensionName) {
+_propertyStore = propertyStore;
+_dimensionPath = ROOT + "/" + dimensionName;
+
+ZNRecord existingRecord = _propertyStore.get(_dimensionPath, null, 
AccessOption.PERSISTENT);
+if (existingRecord == null) {
+  // Create a capacity object using default capacity 
(DEFAULT_CAPACITY_VALUE).
+  _capacity = new ParticipantCapacity(dimensionName);
+} else {
+  _capacity = new ParticipantCapacity(existingRecord);
+}
+  }
+
+  /**
+   * @param zkAddr
+   * @param clusterName
+   * @param dimensionName Identify of the capacity attribute. For example 
memory, CPU.
+   *  Need to match resource quota dimension.
+   */
+  public ZkBasedCapacityProvider(String zkAddr, String clusterName, String 
dimensionName) {
+this(new ZkHelixPropertyStore(zkAddr, new 
ZNRecordSerializer(),
+PropertyPathBuilder.propertyStore(clusterName)), dimensionName);
+  }
+
+  /**
+   * Update capacity information.
+   *
+   * @param capacityMap 
+   * @param usageMap
+   * @param defaultCapacity Default total capacity if not specified in the 
map
+   */
+  public void updateCapacity(Map capacityMap, Map usageMap,
+  int defaultCapacity) {
+for (String participant : capacityMap.keySet()) {
+  _capacity.setCapacity(participant, capacityMap.get(participant));
+}
+for (String participant : usageMap.keySet()) {
+  _capacity.setUsage(participant, usageMap.get(participant));
+}
+_capacity.setDefaultCapacity(defaultCapacity);
+  }
+
+  /**
+   * @return True if the capacity information is successfully wrote to ZK.
+   */
+  public boolean persistCapacity() {
+if (_capacity.isValid()) {
+  return _propertyStore.set(_dimensionPath, _capacity.getRecord(), 
AccessOption.PERSISTENT);
+} else {
+  throw new HelixException("Invalid ParticipantCapacity: " + 
_capacity.getRecord().toString());
+}
+  }
+
+  @Override
+  public int getParticipantCapacity(String participant) {
+return _capacity.getCapacity(participant);
+  }
+
+  @Override
+  public int getParticipantProvisioned(String participant) {
+return _capacity.getUsage(participant);
+  }
+
+  /**
+   * Data model for participant capacity.
+   * Per-participant capacity and usage are recorded in the mapfields.
+   */
+  private static class ParticipantCapaci

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174312501
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/PartitionQuotaProvider.java
 ---
@@ -0,0 +1,33 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting partition quota information.
+ */
+public interface PartitionQuotaProvider {
--- End diff --

if this is providing the weight of a partition, can we call it 
PartitionWeightProvider? Or, more generic, PartitionInfoProvider (and you can 
add other info if you need to)


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174316247
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
+ * apply the change.
+ */
--- End diff --

Good to have extensive javadoc on how to use this tool. Perhaps you  intend 
users to see the tests, in which case you can point to a test on how to use it. 
One example with zk and one without will be ideal. Thanks.


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174313827
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
+ * apply the change.
+ */
+public class WeightAwareRebalanceUtil {
+  private final ClusterConfig _clusterConfig;
+  private final Map _instanceConfigMap = new 
HashMap<>();
+  // For the possible customized state models.
+  private final Map _stateModelDefs = new 
HashMap<>();
+  private final ClusterDataCache _dataCache;
+
+  public enum RebalanceOption {
+INIT,
+REASSIGN
+  }
+
+  public WeightAwareRebalanceUtil(ClusterConfig clusterConfig,
+  List instanceConfigs) {
+for (InstanceConfig instanceConfig : instanceConfigs) {
+  _instanceConfigMap.put(instanceConfig.getInstanceName(), 
instanceConfig);
+}
+_clusterConfig = clusterConfig;
+
+_dataCache = new ClusterDataCache();
+_dataCache.setInstanceConfigMap(_instanceConfigMap);
+_dataCache.setClusterConfig(_clusterConfig);
+List liveInstanceList = new ArrayList<>();
+for (String instance : _instanceConfigMap.keySet()) {
+  LiveInstance liveInstance = new LiveInstance(instance);
+  liveInstanceList.add(liveInstance);
+}
+_dataCache.setLiveInstances(liveInstanceList);
+  }
+
+  /**
+   * The method to generate partition assignment mappings.
+   *
+   * @param resourceConfigsConfig of all the resources that need to be 
rebalanced.
+   *   The tool throws Exception if any resource 
has no IS or broken/uninitialized IS.
+   *   The tool throws Exception if any resource 
is in full-auto mode.
+   * @param existingAssignment The existing partition assignment of the 
input resources.
--- End diff --

s/input resources/resources specified in param resourceConfigs/


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174307432
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/controller/rebalancer/constraint/PartitionWeightAwareEvennessConstraint.java
 ---
@@ -0,0 +1,94 @@
+package org.apache.helix.controller.rebalancer.constraint;
+
+/*
+ * 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.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.PartitionQuotaProvider;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import org.apache.helix.controller.rebalancer.util.ResourceUsageCalculator;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class PartitionWeightAwareEvennessConstraint extends 
AbstractRebalanceSoftConstraint {
+  private final int _weight;
+  private final PartitionQuotaProvider _partitionQuotaProvider;
+  private final CapacityProvider _capacityProvider;
+  // Use to track any assignments that are proposed during the rebalance 
process.
+  // Note these assignments are not reflected in providers.
+  private final Map _pendingUsage;
+
+  public PartitionWeightAwareEvennessConstraint(PartitionQuotaProvider 
partitionQuotaProvider,
+  CapacityProvider capacityProvider, int weight) {
+_partitionQuotaProvider = partitionQuotaProvider;
+_capacityProvider = capacityProvider;
+_weight = weight;
--- End diff --

Not clear what this weight means. Does this mean the relative importance of 
this constraint vs any other? If so, maybe we should consider naming these 
correctly. we have partition weights and constraint weights

Also, _weight is not used in the class?  Should it move to the super class?


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174308429
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/controller/rebalancer/constraint/PartitionWeightAwareEvennessConstraint.java
 ---
@@ -0,0 +1,94 @@
+package org.apache.helix.controller.rebalancer.constraint;
+
+/*
+ * 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.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.PartitionQuotaProvider;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import org.apache.helix.controller.rebalancer.util.ResourceUsageCalculator;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class PartitionWeightAwareEvennessConstraint extends 
AbstractRebalanceSoftConstraint {
+  private final int _weight;
+  private final PartitionQuotaProvider _partitionQuotaProvider;
+  private final CapacityProvider _capacityProvider;
+  // Use to track any assignments that are proposed during the rebalance 
process.
+  // Note these assignments are not reflected in providers.
+  private final Map _pendingUsage;
+
+  public PartitionWeightAwareEvennessConstraint(PartitionQuotaProvider 
partitionQuotaProvider,
+  CapacityProvider capacityProvider, int weight) {
+_partitionQuotaProvider = partitionQuotaProvider;
--- End diff --

add super()


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174308502
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/controller/rebalancer/constraint/TotalCapacityConstraint.java
 ---
@@ -0,0 +1,78 @@
+package org.apache.helix.controller.rebalancer.constraint;
+
+/*
+ * 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.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider;
+import 
org.apache.helix.api.rebalancer.constraint.dataprovider.PartitionQuotaProvider;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import org.apache.helix.controller.rebalancer.util.ResourceUsageCalculator;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class TotalCapacityConstraint extends 
AbstractRebalanceHardConstraint {
+  private final PartitionQuotaProvider _partitionQuotaProvider;
+  private final CapacityProvider _capacityProvider;
+  // Use to track any assignments that are proposed during the rebalance 
process.
+  // Note these assignments are not reflected in providers.
+  private final Map _pendingUsage;
+
+  public TotalCapacityConstraint(PartitionQuotaProvider 
partitionQuotaProvider,
+  CapacityProvider capacityProvider) {
+_partitionQuotaProvider = partitionQuotaProvider;
--- End diff --

add super()


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174313286
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/util/WeightAwareRebalanceUtil.java ---
@@ -0,0 +1,141 @@
+package org.apache.helix.util;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.api.config.RebalanceConfig;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
+import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
+import org.apache.helix.controller.common.PartitionStateMap;
+import org.apache.helix.controller.common.ResourcesStateMap;
+import 
org.apache.helix.controller.rebalancer.strategy.ConstraintRebalanceStrategy;
+import org.apache.helix.controller.stages.ClusterDataCache;
+import org.apache.helix.model.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A rebalance tool that generate an resource partition assignment based 
on the input.
+ * Note the assignment won't be automatically applied to the cluster. 
Applications are supposed to
--- End diff --

s/Application/users of this class/

Eventually, helix will use this class I suppose


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r173588207
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/CapacityProvider.java
 ---
@@ -0,0 +1,38 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting participant capacity information.
+ */
+public interface CapacityProvider {
+
+  /**
+   * @param participant
+   * @return The total participant capacity.
+   */
+  int getParticipantCapacity(String participant);
--- End diff --

If I understand right, this API is to get the current available capacity in 
a participant? If so, can we rename it to getAvailableCapacity(String 
participant). Alternatively (if it helps better in naming) we can name them 
getParticipantCapacity() and getParticipantUsage()


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r173588257
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/CapacityProvider.java
 ---
@@ -0,0 +1,38 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting participant capacity information.
+ */
+public interface CapacityProvider {
+
+  /**
+   * @param participant
+   * @return The total participant capacity.
+   */
+  int getParticipantCapacity(String participant);
+
+  /**
+   * @param participant
+   * @return The provisioned capacity.
+   */
+  int getParticipantProvisioned(String participant);
--- End diff --

Correspondingly, getProvisionedCapacity(String participant)


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[mcvsubbu]

Github user mcvsubbu commented on a diff in the pull request:

https://github.com/apache/helix/pull/145#discussion_r174307115
  
--- Diff: 
helix-core/src/main/java/org/apache/helix/api/rebalancer/constraint/dataprovider/PartitionQuotaProvider.java
 ---
@@ -0,0 +1,33 @@
+package org.apache.helix.api.rebalancer.constraint.dataprovider;
+
+/*
+ * 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.
+ */
+
+/**
+ * An interface for getting partition quota information.
+ */
+public interface PartitionQuotaProvider {
+
+  /**
+   * @param resource
+   * @param partition
+   * @return The quota that is required by a certain partition in the 
specified resource
+   */
+  int getPartitionQuota(String resource, String partition);
--- End diff --

Is this the same as the weight of a partition? If so, can we rename to 
getPartitionWeight()? Just want to make sure that we are consistent with the 
naming (e.g. PartitionWeightAwareEvennessConstraint)


---

[GitHub] helix pull request #145: [HELIX-674] Introducing constraints based rebalanci...

[jiajunwang]

GitHub user jiajunwang opened a pull request:

https://github.com/apache/helix/pull/145

[HELIX-674] Introducing constraints based rebalancing mechanism.

Constraint can be customized by application to restrict how rebalancing is 
processed.
The change also includes examples to demonstrate how constraints can be 
defined and used.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jiajunwang/helix constraint

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/helix/pull/145.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #145


commit 195ca9818617e3909c8bf285912778b711edd67c
Author: jiajunwang 
Date:   2018-02-27T01:21:48Z

[HELIX-674] Introducing constraints based rebalancing mechanism.

Constraint can be customized by application to restrict how rebalancing is 
processed.
The change also includes examples to demonstrate how constraints can be 
defined and used.




---