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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
