klsince commented on code in PR #13256: URL: https://github.com/apache/pinot/pull/13256#discussion_r1638623922
########## pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java: ########## @@ -1611,6 +1622,10 @@ public RealtimeSegmentDataManager(SegmentZKMetadata segmentZKMetadata, TableConf _segmentLogger .info("Starting consumption on realtime consuming segment {} maxRowCount {} maxEndTime {}", llcSegmentName, _segmentMaxRowCount, new DateTime(_consumeEndTime, DateTimeZone.UTC)); + _allowPartialUpsertConsumptionDuringCommit = + _realtimeTableDataManager.isPartialUpsertEnabled() ? _tableConfig.getUpsertConfig() != null Review Comment: the config name as `allowPartialUpsertConsumptionDuringCommit` or `default.allow.partial.upsert.consumption.during.commit` makes a lot sense and it's false by default. but this variable name `_allowPartialUpsertConsumptionDuringCommit` here might be a bit less readable. Here we had to set it true for non-paritial-upsert table. I'd call it `_allowConsumptionDuringCommit`, then it's more intuitive to leave it as true for non-paritial-upsert table ``` _allowConsumptionDuringCommit = !_realtimeTableDataManager.isPartialUpsertEnabled()? true : _tableConfig.getUpsertConfig().isAllowPartialUpsertConsumptionDuringCommit() // nit: no need to check getUpsertConfig() is not null as isPartialUpsertEnabled() is true already. ``` -- 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