PARQUET-99: Add page size check properties #297
Closed
Conversation
This file contains hidden or 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
added 9 commits
July 24, 2015 16:44
|
I like this. ParquetProperties didn't really come into use until later which I think is why v1 is so convoluted. This is a good refactor and will help to expose more configurability as we've been hardcoding many of these properties. |
Contributor
Author
|
Thanks, Dan! Sorry to take so long getting back to this. I'll rebase and clean it up next week and we can get both PARQUET-99 (#250) and this one in for the 1.9.0 release. |
rdblue
force-pushed
the
parquet-properties-update
branch
from
00478f9 to
e072bb9
Compare
Contributor
Author
|
Okay, I've cleaned up my changes on top of #250 and tests are now passing (at least locally). I think the next step is to either cherry-pick e072bb9 into #250 or just update and merge this one. Doesn't matter to me which one. @danielcweeks, could you review my commit on top of yours? @julienledem, you may want to have a look at the API changes as well. Thanks! |
Member
|
@rdblue This is a much needed cleanup and refactoring. Thanks for doing this! I made some comment above. Otherwise this looks good to me. |
rdblue
force-pushed
the
parquet-properties-update
branch
from
e072bb9 to
c93b73e
Compare
Contributor
Author
|
Updated for Julien's comments. Thanks for reviewing! |
Member
|
+1 |
rdblue
changed the title
piyushnarang
pushed a commit
to piyushnarang/parquet-mr
that referenced
this pull request
Jun 15, 2016
This adds properties to set the min and max number of records that are passed between page checks, as well as a property that controls whether the next check will be based on records already seen or set to the minimum number of records between checks. * `parquet.page.size.row.check.min` - minimum number of records between page size checks * `parquet.page.size.row.check.max` - maximum number of records between page size checks * `parquet.page.size.check.estimate` - whether to estimate the number of records before the next check, or to always use the minimum number of records. This also updates the internal API to use ParquetProperties to carry encoding settings (used in parquet-column) to reduce the number of parameters passed through internal APIs. It also adds a builder for ParquetProperties to avoid needing to reference defaults in other modules. This closes apache#250 Author: Daniel Weeks <dweeks@netflix.com> Author: Ryan Blue <blue@apache.org> Closes apache#297 from rdblue/parquet-properties-update and squashes the following commits: c93b73e [Ryan Blue] PARQUET-99: Use ParquetProperties to carry encoding config. 18f8d3a [Daniel Weeks] Spacing 2090719 [Daniel Weeks] Update sizeCheck to write page properly if estimating is turned off 71336ee [Daniel Weeks] Fixed param name 5d99072 [Daniel Weeks] Update page size checking for v2 writer 3f7870c [Daniel Weeks] Rebase to resolve byte buffer conflicts 68794f0 [Daniel Weeks] Merge branch 'master' into page_size_check b49f03c [Daniel Weeks] Fixed reset of nextSizeCheck a057f46 [Daniel Weeks] Fixed inverted property logic e7cd54b [Daniel Weeks] Added property to toggle page size check estimation and initial row size checking
rdblue
pushed a commit
to rdblue/parquet-mr
that referenced
this pull request
Jul 13, 2016
This adds properties to set the min and max number of records that are passed between page checks, as well as a property that controls whether the next check will be based on records already seen or set to the minimum number of records between checks. * `parquet.page.size.row.check.min` - minimum number of records between page size checks * `parquet.page.size.row.check.max` - maximum number of records between page size checks * `parquet.page.size.check.estimate` - whether to estimate the number of records before the next check, or to always use the minimum number of records. This also updates the internal API to use ParquetProperties to carry encoding settings (used in parquet-column) to reduce the number of parameters passed through internal APIs. It also adds a builder for ParquetProperties to avoid needing to reference defaults in other modules. This closes apache#250 Author: Daniel Weeks <dweeks@netflix.com> Author: Ryan Blue <blue@apache.org> Closes apache#297 from rdblue/parquet-properties-update and squashes the following commits: c93b73e [Ryan Blue] PARQUET-99: Use ParquetProperties to carry encoding config. 18f8d3a [Daniel Weeks] Spacing 2090719 [Daniel Weeks] Update sizeCheck to write page properly if estimating is turned off 71336ee [Daniel Weeks] Fixed param name 5d99072 [Daniel Weeks] Update page size checking for v2 writer 3f7870c [Daniel Weeks] Rebase to resolve byte buffer conflicts 68794f0 [Daniel Weeks] Merge branch 'master' into page_size_check b49f03c [Daniel Weeks] Fixed reset of nextSizeCheck a057f46 [Daniel Weeks] Fixed inverted property logic e7cd54b [Daniel Weeks] Added property to toggle page size check estimation and initial row size checking Conflicts: parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java parquet-column/src/main/java/org/apache/parquet/column/impl/ColumnWriteStoreV1.java parquet-column/src/main/java/org/apache/parquet/column/impl/ColumnWriteStoreV2.java parquet-column/src/main/java/org/apache/parquet/column/impl/ColumnWriterV1.java parquet-column/src/main/java/org/apache/parquet/column/impl/ColumnWriterV2.java parquet-column/src/test/java/org/apache/parquet/column/impl/TestCorruptDeltaByteArrays.java parquet-column/src/test/java/org/apache/parquet/column/mem/TestMemColumn.java parquet-column/src/test/java/org/apache/parquet/io/PerfTest.java parquet-column/src/test/java/org/apache/parquet/io/TestColumnIO.java parquet-column/src/test/java/org/apache/parquet/io/TestFiltered.java parquet-hadoop/src/main/java/org/apache/parquet/hadoop/InternalParquetRecordWriter.java parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetOutputFormat.java parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetRecordWriter.java parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetWriter.java parquet-pig/src/test/java/org/apache/parquet/pig/TupleConsumerPerfTest.java parquet-thrift/src/test/java/org/apache/parquet/thrift/TestParquetReadProtocol.java Resolution: Fixed changes that depended on the addition of an allocator argument Ignored adjacent changes that were flagged Passed page size at compressor instantiation instead of to the factory
rdblue
pushed a commit
to rdblue/parquet-mr
that referenced
this pull request
Jan 6, 2017
This adds properties to set the min and max number of records that are passed between page checks, as well as a property that controls whether the next check will be based on records already seen or set to the minimum number of records between checks. * `parquet.page.size.row.check.min` - minimum number of records between page size checks * `parquet.page.size.row.check.max` - maximum number of records between page size checks * `parquet.page.size.check.estimate` - whether to estimate the number of records before the next check, or to always use the minimum number of records. This also updates the internal API to use ParquetProperties to carry encoding settings (used in parquet-column) to reduce the number of parameters passed through internal APIs. It also adds a builder for ParquetProperties to avoid needing to reference defaults in other modules. This closes apache#250 Author: Daniel Weeks <dweeks@netflix.com> Author: Ryan Blue <blue@apache.org> Closes apache#297 from rdblue/parquet-properties-update and squashes the following commits: c93b73e [Ryan Blue] PARQUET-99: Use ParquetProperties to carry encoding config. 18f8d3a [Daniel Weeks] Spacing 2090719 [Daniel Weeks] Update sizeCheck to write page properly if estimating is turned off 71336ee [Daniel Weeks] Fixed param name 5d99072 [Daniel Weeks] Update page size checking for v2 writer 3f7870c [Daniel Weeks] Rebase to resolve byte buffer conflicts 68794f0 [Daniel Weeks] Merge branch 'master' into page_size_check b49f03c [Daniel Weeks] Fixed reset of nextSizeCheck a057f46 [Daniel Weeks] Fixed inverted property logic e7cd54b [Daniel Weeks] Added property to toggle page size check estimation and initial row size checking Conflicts: parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java parquet-column/src/main/java/org/apache/parquet/column/impl/ColumnWriteStoreV1.java parquet-column/src/main/java/org/apache/parquet/column/impl/ColumnWriteStoreV2.java parquet-column/src/main/java/org/apache/parquet/column/impl/ColumnWriterV1.java parquet-column/src/main/java/org/apache/parquet/column/impl/ColumnWriterV2.java parquet-column/src/test/java/org/apache/parquet/column/impl/TestCorruptDeltaByteArrays.java parquet-column/src/test/java/org/apache/parquet/column/mem/TestMemColumn.java parquet-column/src/test/java/org/apache/parquet/io/PerfTest.java parquet-column/src/test/java/org/apache/parquet/io/TestColumnIO.java parquet-column/src/test/java/org/apache/parquet/io/TestFiltered.java parquet-hadoop/src/main/java/org/apache/parquet/hadoop/InternalParquetRecordWriter.java parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetOutputFormat.java parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetRecordWriter.java parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetWriter.java parquet-pig/src/test/java/org/apache/parquet/pig/TupleConsumerPerfTest.java parquet-thrift/src/test/java/org/apache/parquet/thrift/TestParquetReadProtocol.java Resolution: Fixed changes that depended on the addition of an allocator argument Ignored adjacent changes that were flagged Passed page size at compressor instantiation instead of to the factory
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This adds properties to set the min and max number of records that are passed between page checks, as well as a property that controls whether the next check will be based on records already seen or set to the minimum number of records between checks.
parquet.page.size.row.check.min- minimum number of records between page size checksparquet.page.size.row.check.max- maximum number of records between page size checksparquet.page.size.check.estimate- whether to estimate the number of records before the next check, or to always use the minimum number of records.This also updates the internal API to use ParquetProperties to carry encoding settings (used in parquet-column) to reduce the number of parameters passed through internal APIs. It also adds a builder for ParquetProperties to avoid needing to reference defaults in other modules.
This closes #250