imply-cheddar commented on code in PR #13070:
URL: https://github.com/apache/druid/pull/13070#discussion_r971469785
##########
server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java:
##########
@@ -848,6 +848,11 @@ private SegmentIdWithShardSpec createNewSegment(
versionOfExistingChunk = null;
}
+ // The number of core partitions must always be chosen from the set of
used segments in the VersionedIntervalTimeline.
+ // When the core partitions have been dropped, using pending segments
may lead to an incorrect state
+ // where the chunk is believed to have core partitions and queries
results are incorrect.
+ int numCorePartitions = maxId == null ? 0 :
maxId.getShardSpec().getNumCorePartitions();
Review Comment:
The change as written requires an understanding that the `maxId` variable is
going to be replaced to understand that this specific position of this line is
very important for correctness. I might recommend that we maintain a
`commitedMaxId` and a `overallMaxId` as two separate variables and then use the
correct one down at the bottom. This makes it more clear what the state that
we are depending on is.
--
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]
