[GitHub] [incubator-druid] dclim commented on a change in pull request #7514: Introduce segment limit for indexing tasks

[GitBox <gi...@apache.org>]

dclim commented on a change in pull request #7514: Introduce segment limit for indexing tasks
URL: https://github.com/apache/incubator-druid/pull/7514#discussion_r279088637
 
 

 ##########
 File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/AppenderatorDriverRealtimeIndexTask.java
 ##########
 @@ -313,6 +316,23 @@ public TaskStatus run(final TaskToolbox toolbox)
             log.debug("Discarded null row, considering thrownAway.");
             rowIngestionMeters.incrementThrownAway();
           } else {
+            // Check if the upcoming row will result in the creation of a new segment.  If so and the new number of segments
 
 Review comment:
   I don't love the added complexity and the code duplication (this similar block is repeated 4 times) introduced by trying to pre-determine whether the row will cause a new segment to be generated before it's sent to the appenderator. Also, `AppenderatorDriverAddResult.isPushRequired()` feels like the right place for this decision to be made, and not in every indexing task type that uses an appenderator.
   
   I understand that after `Stream/BatchAppenderatorDriver.add()` is called, if you determine that you're now over `maxTotalSegments`, you wind up pushing a segment that has only one row. But that is only true if you try to push _all_ of the open segments. What if instead you made appropriate modifications to methods like `StreamAppenderatorDriver.publish()` and `BatchAppenderatorDriver.pushAllAndClear()` (or introduced new methods) that allow you to provide a list of segmentIds to exclude, which you would use to exclude the newly created segment that pushed you over the limit?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org