KKcorps commented on code in PR #20007:
URL: https://github.com/apache/pinot/pull/20007#discussion_r4356543900
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentIndexCreationDriverImpl.java:
##########
@@ -296,14 +297,23 @@ public void build()
for (GenericRow row : result.getTransformedRows()) {
_indexCreator.indexRow(row);
+ numDocsIndexed++;
}
_totalIndexTimeNs += System.nanoTime() - recordReadStopTimeNs;
_incompleteRowsFound += result.getIncompleteRowCount();
_skippedRowsFound += result.getSkippedRowCount();
_sanitizedRowsFound += result.getSanitizedRowCount();
}
+ // The stats pass sized the dictionaries and wrote total docs into the
metadata. If the index pass read a
+ // different number of rows, some dictionary ids own no document and the
indexes disagree with the metadata.
+ if (numDocsIndexed != _totalDocs) {
+ throw new IllegalStateException(
+ "Segment: " + _config.getSegmentName() + " of table: " +
_config.getTableName() + " indexed "
Review Comment:
Thanks, I added a comment above the check in 4fe587e. It says only the row
based path rereads the record reader, and the sorted forward index `close()`
check still covers `buildByColumn()` and `buildColumnar()`.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentIndexCreationDriverImpl.java:
##########
@@ -296,14 +297,23 @@ public void build()
for (GenericRow row : result.getTransformedRows()) {
_indexCreator.indexRow(row);
+ numDocsIndexed++;
}
_totalIndexTimeNs += System.nanoTime() - recordReadStopTimeNs;
_incompleteRowsFound += result.getIncompleteRowCount();
_skippedRowsFound += result.getSkippedRowCount();
_sanitizedRowsFound += result.getSanitizedRowCount();
}
Review Comment:
Good point, I added this to the PR description. With `continueOnError`, a
reader that drops different rows on each pass now fails the build. Before, it
could seal a segment whose indexes disagree with its metadata.
--
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]
