GitHub user jackylk opened a pull request:
https://github.com/apache/carbondata/pull/2682 [CARBONDATA-2907] Support setting blocklet size in table property When creating table, should support setting blocklet size (property name "TABLE_BLOCKLET_SIZE"). If user does not set this table property, use system level property - [X] Any interfaces changed? No - [X] Any backward compatibility impacted? No - [X] Document update required? Yes, updated - [X] Testing done Please provide details on - Whether new unit test cases have been added or why no new tests are required? - How it is tested? Please attach test report. - Is it a performance related change? Please attach the performance test report. - Any additional information to help reviewers in testing this change. two testcase added - [X] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA. NA You can merge this pull request into a Git repository by running: $ git pull https://github.com/jackylk/incubator-carbondata fix_blockletsize Alternatively you can review and apply these changes as the patch at: https://github.com/apache/carbondata/pull/2682.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2682 ---- commit ff85ebb420e67ff6400ff1e26e565f030d9d7f56 Author: Jacky Li <jacky.likun@...> Date: 2018-09-01T16:32:29Z support blocklet size table property ---- --- |
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2682 Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/154/ --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2682 Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8225/ --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2682 Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8228/ --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2682 Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/157/ --- |
In reply to this post by qiuchenjian-2
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2682#discussion_r214629556 --- Diff: processing/src/main/java/org/apache/carbondata/processing/store/writer/v3/CarbonFactDataWriterImplV3.java --- @@ -68,11 +70,14 @@ public CarbonFactDataWriterImplV3(CarbonFactDataHandlerModel model) { super(model); - blockletSizeThreshold = Long.parseLong(CarbonProperties.getInstance() - .getProperty(CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB, - CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB_DEFAULT_VALUE)) - * CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR - * CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR; + String blockletSize = --- End diff -- I didn't see the code to verify this value's correctness --- |
In reply to this post by qiuchenjian-2
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2682#discussion_r214630184 --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilder.java --- @@ -117,7 +116,7 @@ public TableSchema build() { property.put(CarbonCommonConstants.TABLE_BLOCKSIZE, String.valueOf(blockSize)); } if (blockletSize > 0) { - property.put(CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB, String.valueOf(blockletSize)); --- End diff -- Why not use the old variable directly? --- |
In reply to this post by qiuchenjian-2
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2682#discussion_r214629195 --- Diff: processing/src/main/java/org/apache/carbondata/processing/store/writer/v3/CarbonFactDataWriterImplV3.java --- @@ -68,11 +70,14 @@ public CarbonFactDataWriterImplV3(CarbonFactDataHandlerModel model) { super(model); - blockletSizeThreshold = Long.parseLong(CarbonProperties.getInstance() - .getProperty(CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB, - CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB_DEFAULT_VALUE)) - * CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR - * CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR; + String blockletSize = + model.getTableSpec().getCarbonTable().getTableInfo().getFactTable().getTableProperties() + .get(TABLE_BLOCKLET_SIZE); + if (blockletSize == null) { --- End diff -- If the user does not specify the blocklet size and the blocklet size is changed during the loading. Will it be a problem? --- |
In reply to this post by qiuchenjian-2
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2682#discussion_r214705354 --- Diff: processing/src/main/java/org/apache/carbondata/processing/store/writer/v3/CarbonFactDataWriterImplV3.java --- @@ -68,11 +70,14 @@ public CarbonFactDataWriterImplV3(CarbonFactDataHandlerModel model) { super(model); - blockletSizeThreshold = Long.parseLong(CarbonProperties.getInstance() - .getProperty(CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB, - CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB_DEFAULT_VALUE)) - * CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR - * CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR; + String blockletSize = + model.getTableSpec().getCarbonTable().getTableInfo().getFactTable().getTableProperties() + .get(TABLE_BLOCKLET_SIZE); + if (blockletSize == null) { --- End diff -- not a problem. blocklet size is get from the property for every load --- |
In reply to this post by qiuchenjian-2
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2682#discussion_r214706037 --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilder.java --- @@ -117,7 +116,7 @@ public TableSchema build() { property.put(CarbonCommonConstants.TABLE_BLOCKSIZE, String.valueOf(blockSize)); } if (blockletSize > 0) { - property.put(CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB, String.valueOf(blockletSize)); --- End diff -- CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB is "carbon.blockletgroup.size.in.mb", this string is system property, but a normal table property is without dot, like `TABLE_BLOCKSIZE = "table_blocksize"; So I added a `TABLE_BLOCKLET_SIZE = "table_blocklet_size"` --- |
In reply to this post by qiuchenjian-2
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2682#discussion_r214710189 --- Diff: processing/src/main/java/org/apache/carbondata/processing/store/writer/v3/CarbonFactDataWriterImplV3.java --- @@ -68,11 +70,14 @@ public CarbonFactDataWriterImplV3(CarbonFactDataHandlerModel model) { super(model); - blockletSizeThreshold = Long.parseLong(CarbonProperties.getInstance() - .getProperty(CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB, - CarbonV3DataFormatConstants.BLOCKLET_SIZE_IN_MB_DEFAULT_VALUE)) - * CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR - * CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR; + String blockletSize = --- End diff -- I added in CarbonDDLSqlParser. And one testcase is added --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2682 Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8263/ --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2682 Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/192/ --- |
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2682 LGTM --- |
In reply to this post by qiuchenjian-2
|
Free forum by Nabble | Edit this page |