Copilot commented on code in PR #17556:
URL: https://github.com/apache/pinot/pull/17556#discussion_r2722453860
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -1004,6 +1004,9 @@ public Collection<String>
getLastLLCCompletedSegments(String tableNameWithType)
for (SegmentZKMetadata zkMetadata :
getSegmentsZKMetadata(tableNameWithType)) {
if (zkMetadata.getStatus() ==
CommonConstants.Segment.Realtime.Status.DONE) {
LLCSegmentName llcName =
LLCSegmentName.of(zkMetadata.getSegmentName());
+ if (llcName == null) {
+ continue;
+ }
Review Comment:
A null check for `llcName` is added in the original
`getLastLLCCompletedSegments()` method but this method was not previously
checking for null. This changes the behavior of the existing method. The null
check should only be in the new overloaded method to avoid altering the
behavior of the existing API.
```suggestion
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java:
##########
@@ -168,7 +172,7 @@ private void manageRetentionForTable(TableConfig
tableConfig) {
// TODO: handle the orphan segment deletion for hybrid table
manageRetentionForHybridTable(tableConfig, offlineTableConfig);
} else {
- manageRetentionForRealtimeTable(tableNameWithType, retentionStrategy,
untrackedSegmentsDeletionBatchSize,
+ manageRetentionForRealtimeTable(tableConfig, retentionStrategy,
untrackedSegmentsDeletionBatchSize,
Review Comment:
The method signature of `manageRetentionForRealtimeTable` changed from
accepting `String realtimeTableName` to `TableConfig tableConfig`. While the
new parameter is more flexible, this is a breaking change to the method
signature. Consider deprecating the old signature if this is meant to be part
of the public API, or document this as a breaking change.
--
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]
