nwangtw commented on a change in pull request #3187: Consolidate Packing
URL: https://github.com/apache/incubator-heron/pull/3187#discussion_r255194337
 
 

 ##########
 File path: 
heron/packing/src/java/org/apache/heron/packing/utils/PackingUtils.java
 ##########
 @@ -26,74 +26,42 @@
 import org.apache.heron.api.generated.TopologyAPI;
 import org.apache.heron.api.utils.TopologyUtils;
 import org.apache.heron.common.basics.ByteAmount;
-import org.apache.heron.spi.packing.PackingException;
 import org.apache.heron.spi.packing.Resource;
 
 /**
  * Shared utilities for packing algorithms
  */
 public final class PackingUtils {
   private static final Logger LOG = 
Logger.getLogger(PackingUtils.class.getName());
-  private static final ByteAmount MIN_RAM_PER_INSTANCE = 
ByteAmount.fromMegabytes(192);
+
+  // default
+  public static final int DEFAULT_CONTAINER_PADDING_PERCENTAGE = 10;
+  public static final ByteAmount DEFAULT_CONTAINER_RAM_PADDING = 
ByteAmount.fromGigabytes(1);
+  public static final ByteAmount DEFAULT_CONTAINER_DISK_PADDING = 
ByteAmount.fromGigabytes(1);
+  public static final double DEFAULT_CONTAINER_CPU_PADDING = 1.0;
+  public static final int DEFAULT_MAX_NUM_INSTANCES_PER_CONTAINER = 4;
 
   private PackingUtils() {
   }
 
-  /**
-   * Verifies the Instance has enough RAM and that it can fit within the 
container limits.
-   *
-   * @param instanceResources The resources allocated to the instance
-   * @throws PackingException if the instance is invalid
-   */
-  private static void assertIsValidInstance(Resource instanceResources,
-                                            ByteAmount minInstanceRam,
-                                            Resource maxContainerResources,
-                                            int paddingPercentage) throws 
PackingException {
-
-    if (instanceResources.getRam().lessThan(minInstanceRam)) {
-      throw new PackingException(String.format(
-          "Instance requires RAM %s which is less than the minimum RAM per 
instance of %s",
-          instanceResources.getRam(), minInstanceRam));
-    }
-
-    ByteAmount instanceRam = 
instanceResources.getRam().increaseBy(paddingPercentage);
-    if (instanceRam.greaterThan(maxContainerResources.getRam())) {
-      throw new PackingException(String.format(
-          "This instance requires containers of at least %s RAM. The current 
max container "
-              + "size is %s",
-          instanceRam, maxContainerResources.getRam()));
+  public static Map<String, Resource> getComponentResourceMap(
 
 Review comment:
   Add comments for this function.
   
   parallelismMap is used to get component list? Suggest to use the list 
instead to be more explicit.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to