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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to