mcvsubbu commented on a change in pull request #7058:
URL: https://github.com/apache/incubator-pinot/pull/7058#discussion_r669006297



##########
File path: 
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -539,15 +543,42 @@ private void commitSegmentMetadataInternal(String 
realtimeTableName,
       lock.unlock();
     }
 
-    // TODO: also create the new partition groups here, instead of waiting 
till the {@link RealtimeSegmentValidationManager} runs
+    //  Creates new partition groups here instead of waiting till the {@link 
RealtimeSegmentValidationManager} runs
     //  E.g. If current state is A, B, C, and newPartitionGroupMetadataList 
contains B, C, D, E,
-    //  then create metadata/idealstate entries for D, E along with the 
committing partition's entries.
-    //  Ensure that multiple committing segments don't create multiple new 
segment metadata and ideal state entries for the same partitionGroup
+    //  then metadata/idealstate entries for D, E are created along with the 
committing partition's entries.
+
+    addNewPartitionGroups(realtimeTableName, tableConfig, instancePartitions, 
idealState, numReplicas, streamConfig,
+        newPartitionGroupMetadataList, numPartitionGroups, segmentAssignment, 
instancePartitionsMap);
 
     // Trigger the metadata event notifier
     _metadataEventNotifierFactory.create().notifyOnSegmentFlush(tableConfig);
   }
 
+  /**
+   * Method is kept synchronised so that multiple committing segments don't 
create multiple new segment metadata
+   * and ideal state entries for the same partitionGroup
+   */
+  private synchronized void addNewPartitionGroups(String realtimeTableName, 
TableConfig tableConfig,

Review comment:
       In that case, if the background job beats the segment commit logic, then 
there will already be a consuming segment created for the new partition, and a 
call to updateInstanceStatesaForNewConsumingSegment() will result in an 
exception. Just be sure to handle the exception correctly. It is better to 
throw a specific exception, so that you can catch that particular exception 
here.




-- 
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-unsubscr...@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to