You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@samza.apache.org by GitBox <gi...@apache.org> on 2022/02/01 00:11:32 UTC

[GitHub] [samza] lakshmi-manasa-g commented on a change in pull request #1576: SAMZA-2688 [Elasticity] introduce elasticity factor config and key bucket within SSP

lakshmi-manasa-g commented on a change in pull request #1576:
URL: https://github.com/apache/samza/pull/1576#discussion_r796171042



##########
File path: samza-api/src/main/java/org/apache/samza/system/SystemStreamPartition.java
##########
@@ -38,6 +39,7 @@ public SystemStreamPartition(String system, String stream, Partition partition)
     super(system, stream);
     this.partition = partition;
     this.hash = computeHashCode();
+    this.keyBucket = -1;

Review comment:
       agreed that keeping keyBucket = 0 when there is no elasticity makes this particular SSP class simpler.
   
   my reason for having it to -1 was as follows.
   making keyBucket = 0 and removing the check `keyBucket == -1` means every time SSP is printed into logs and task model, it will be `SystemStreamPartition [system, stream, partition, 0] with no elasticity enabled which can be confusing.
   
   this check of `keyBucket-1` will be restricted to this class and hence i think it is better to have this additional check rather than having to introduce SSP containing 0 in all places. 
   
   pl lmk if this makes sense

##########
File path: samza-core/src/main/java/org/apache/samza/config/JobConfig.java
##########
@@ -466,4 +472,12 @@ public boolean getStartpointEnabled() {
   public boolean getContainerHeartbeatMonitorEnabled() {
     return getBoolean(CONTAINER_HEARTBEAT_MONITOR_ENABLED, CONTAINER_HEARTBEAT_MONITOR_ENABLED_DEFAULT);
   }
+
+  public boolean getElasticityEnabled() {

Review comment:
       is the question why `getElasticityEnabled` is needed when we have `getElasticityFactor`?
   callers wanting to know if elasticity is enabled need not and should not be required to know that elasticity.factor = 1 means disabled and >1 means enabled. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@samza.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org