kfaraz commented on code in PR #15169:
URL: https://github.com/apache/druid/pull/15169#discussion_r1361105395
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -1093,6 +1093,21 @@ public void resetOffsets(@Nonnull DataSourceMetadata
resetDataSourceMetadata)
addNotice(new ResetOffsetsNotice(resetDataSourceMetadata));
}
+ @Override
Review Comment:
Would be nice to add a javadoc here or in the interface to say a few words
about the base sequence name and what kind of strings we can expect in it.
You could also link to the javadoc of `TaskGroup.baseSequenceName` if one
already exists. If it doesn't, maybe add a getter for this field
`TaskGroup.getBaseSequenceName()` and write a small javadoc for it.
##########
extensions-contrib/materialized-view-maintenance/src/main/java/org/apache/druid/indexing/materializedview/MaterializedViewSupervisor.java:
##########
@@ -295,6 +295,12 @@ public LagStats computeLagStats()
throw new UnsupportedOperationException("Compute Lag Stats not supported
in MaterializedViewSupervisor");
}
+ @Override
+ public Set<String> getActiveBaseSequenceNames()
+ {
+ throw new UnsupportedOperationException("Get Active sequence names is not
supported in MaterializedViewSupervisor");
Review Comment:
Nit: Probably don't need an explicit message here.
##########
server/src/main/java/org/apache/druid/indexing/overlord/IndexerMetadataStorageCoordinator.java:
##########
@@ -345,10 +345,12 @@ SegmentPublishResult commitReplaceSegments(
* </ul>
*
* @param replaceSegments Segments being committed by a REPLACE task
+ * @param activeBaseSequenceNames of base sequence names of active / pending
completion task groups of the supervisor
Review Comment:
```suggestion
* @param activeBaseSequenceNames Set of base sequence names of active and
pending completion task groups of the supervisor (if any) for this datasource
```
##########
server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java:
##########
@@ -660,6 +662,18 @@ private Map<SegmentIdWithShardSpec,
SegmentIdWithShardSpec> upgradePendingSegmen
: overlappingPendingSegments.entrySet()) {
final SegmentIdWithShardSpec pendingSegmentId =
overlappingPendingSegment.getKey();
final String pendingSegmentSequence =
overlappingPendingSegment.getValue();
+
+ boolean considerSequence = false;
Review Comment:
Based on the offline discussion, I think we had decided to include the
sequence name in the SELECT query itself, rather than filtering in-memory here.
--
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]
