DRILL-7240: Catch runtime pruning filter-match exceptions and do not prune these rowgroups #1783
+26
−11
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amansinha100
reviewed
May 6, 2019
| matchResult == RowsMatch.NONE ? "Excluded" : "Included", rowGroup.getPath(), rowGroupIndex, footerRowCount, timeToRead); | ||
| } catch (ClassCastException cce) { | ||
| if ( ! matchCastErrorNotified ) { | ||
| logger.info("Run-time pruning check failed due to type casting. Skipping pruning rowgroups starting from {}. (Error: {})", rowGroup.getPath(), cce.getMessage()); |
There was a problem hiding this comment.
Given the current behavior with non-interesting columns, there could be thousands of row groups which have this same ClassCastException. It would be better to consolidate and generate a single INFO message.
There was a problem hiding this comment.
Done, here is a sample from the log (tested with 6 rowgroups/files; three of which fail, or the remaining three one was pruned):
2019-05-06 18:37:19,809 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] TRACE o.a.d.e.s.p.AbstractParquetScanBatchCreator - ParquetTrace,Read Footer,,/tmp/twofoo/sub/0_0_3.parquet,,0,0,0,170059
2019-05-06 18:37:19,828 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] TRACE o.a.d.e.s.p.AbstractParquetScanBatchCreator - ParquetTrace,Read Footer,,/tmp/twofoo/sub/0_0_2.parquet,,0,0,0,4691
2019-05-06 18:37:19,829 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] TRACE o.a.d.e.s.p.AbstractParquetScanBatchCreator - Run-time pruning: Included row-group /tmp/twofoo/sub/0_0_2.parquet (RG index: 0 row count: 2), took 466 usec
2019-05-06 18:37:19,869 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] TRACE o.a.d.e.s.p.AbstractParquetScanBatchCreator - ParquetTrace,Read Footer,,/tmp/twofoo/sub/0_0_0.parquet,,0,0,0,37316
2019-05-06 18:37:19,870 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] TRACE o.a.d.e.s.p.AbstractParquetScanBatchCreator - Run-time pruning: Excluded row-group /tmp/twofoo/sub/0_0_0.parquet (RG index: 0 row count: 2), took 683 usec
2019-05-06 18:37:19,909 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] TRACE o.a.d.e.s.p.AbstractParquetScanBatchCreator - ParquetTrace,Read Footer,,/tmp/twofoo/sub/0_0_1.parquet,,0,0,0,38874
2019-05-06 18:37:19,914 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] TRACE o.a.d.e.s.p.AbstractParquetScanBatchCreator - ParquetTrace,Read Footer,,/tmp/twofoo/sub/0_0_4.parquet,,0,0,0,2417
2019-05-06 18:37:19,915 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] TRACE o.a.d.e.s.p.AbstractParquetScanBatchCreator - Run-time pruning: Included row-group /tmp/twofoo/sub/0_0_4.parquet (RG index: 0 row count: 2), took 356 usec
2019-05-06 18:37:19,919 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] TRACE o.a.d.e.s.p.AbstractParquetScanBatchCreator - ParquetTrace,Read Footer,,/tmp/twofoo/sub/0_0_5.parquet,,0,0,0,2749
2019-05-06 18:37:19,922 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] INFO o.a.d.e.s.p.AbstractParquetScanBatchCreator - Finished parquet_runtime_pruning in 1505 usec. Out of given 6 rowgroups, 1 were pruned.
2019-05-06 18:37:19,922 [232f1eb3-2cab-7d16-1222-c05eca0fdceb:frag:0:0] INFO o.a.d.e.s.p.AbstractParquetScanBatchCreator - Run-time pruning check skipped for 3 out of 6 rowgroups due to: java.lang.Integer cannot be cast to java.lang.Long
amansinha100
reviewed
May 7, 2019
| if ( totalPruneTime > 0 ) { | ||
| logger.info("Finished parquet_runtime_pruning in {} usec. Out of given {} rowgroups, {} were pruned. {}", totalPruneTime, totalRowgroups, rowgroupsPruned, | ||
| totalRowgroups == rowgroupsPruned ? "ALL_PRUNED !!" : ""); | ||
| } | ||
| if ( countMatchClassCastExceptions > 0 ) { | ||
| logger.info("Run-time pruning check skipped for {} out of {} rowgroups due to: {}",countMatchClassCastExceptions, totalRowgroups, matchCastErrorMessage); |
There was a problem hiding this comment.
Minor point: The word 'check' should be removed since at least a partial check was performed and the failure happened during that process.
amansinha100
approved these changes
May 7, 2019
…prune these rowgroups
lushuifeng
pushed a commit
to lushuifeng/drill
that referenced
this pull request
Jun 21, 2019
…prune these rowgroups closes apache#1783
xiangt920
pushed a commit
to xiangt920/drill
that referenced
this pull request
Dec 26, 2019
…prune these rowgroups closes apache#1783
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The basic change is simple: Catch exceptions thrown by the
match()method checking the filter predicate over a rowgroup's statistics, and just avoid pruning that rowgroup.The current known case of such exception has to do with type casting (Long-Integer). In that case an INFO message is logged (only once per fragment). Note the case of mixed rowgroups/files, where some have the "correct" type and some don't - in that case the rowgroups with the "correct" type (i.e. no exception) would be pruned as needed (I tested that).
In case other exception ever shows up, it would log a WARN message (so eventually this failure could be analyzed).