klsince commented on code in PR #15268:
URL: https://github.com/apache/pinot/pull/15268#discussion_r1996097154
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -1725,6 +1723,19 @@ public RealtimeSegmentDataManager(SegmentZKMetadata
segmentZKMetadata, TableConf
}
}
+ // Consumption while downloading and replacing the slow replicas is not
allowed for the following use cases:
+ // 1. Partial Upserts
+ // 2. Dedup Tables
+ // For the above table types, we would be looking into the metadata
information when inserting a new record,
+ // so it is not ideal to allow consumption when downloading and replacing
the consuming segment as we might see
+ // unusual behaviour of duplicates in dedup tables and inconsistent entries
compared to lead replicas for partial
+ // upsert tables.
+ private boolean isConsumptionAllowedDuringCommit() {
+ return (!_realtimeTableDataManager.isPartialUpsertEnabled() &&
!_realtimeTableDataManager.isDedupEnabled()) || (
+ _tableConfig.getUpsertConfig() != null &&
_tableConfig.getUpsertConfig()
+ .isAllowPartialUpsertConsumptionDuringCommit());
Review Comment:
maybe this? so it's more clear that
isAllowPartialUpsertConsumptionDuringCommit is only used for upsert table.
```
return !_realtimeTableDataManager.isDedupEnabled() &&
(!_realtimeTableDataManager.isPartialUpsertEnabled() ||
_tableConfig.getUpsertConfig().isAllowPartialUpsertConsumptionDuringCommit())
```
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -1725,6 +1723,19 @@ public RealtimeSegmentDataManager(SegmentZKMetadata
segmentZKMetadata, TableConf
}
}
+ // Consumption while downloading and replacing the slow replicas is not
allowed for the following use cases:
+ // 1. Partial Upserts
+ // 2. Dedup Tables
+ // For the above table types, we would be looking into the metadata
information when inserting a new record,
+ // so it is not ideal to allow consumption when downloading and replacing
the consuming segment as we might see
+ // unusual behaviour of duplicates in dedup tables and inconsistent entries
compared to lead replicas for partial
+ // upsert tables.
Review Comment:
nit: Consumption while downloading and replacing the `consuming segment` on
slow replicas ...
nit: ... so it's not right to allow consumption .. as we might see
duplicates in dedup tables or inconsistent entries in partial upsert table
--
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]
