You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by akashrn5 <gi...@git.apache.org> on 2018/08/27 06:28:06 UTC
[GitHub] carbondata pull request #2662: [CARBONDATA-2889]Add decoder based fallback m...
GitHub user akashrn5 opened a pull request:
https://github.com/apache/carbondata/pull/2662
[CARBONDATA-2889]Add decoder based fallback mechanism in local dictionary to reduce memory footprint
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
- [ ] Any backward compatibility impacted?
- [ ] Document update required?
- [ ] 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.
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/akashrn5/incubator-carbondata fallback
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2662.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 #2662
----
commit 8729c673ed06ee16ed256270e31c495bd1568bfd
Author: akashrn5 <ak...@...>
Date: 2018-08-20T04:59:26Z
Add decoder based fallback mechanism in local dictionary to reduce memory footprint
----
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/18/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on the issue:
https://github.com/apache/carbondata/pull/2662
LGTM
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8235/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.3/8336/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/358/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/149/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.3/8352/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2662
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6465/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/262/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8083/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2662
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6480/
---
[GitHub] carbondata pull request #2662: [WIP][CARBONDATA-2889]Add decoder based fallb...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r213161218
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/FallbackDecoderBasedColumnPageEncoder.java ---
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.carbondata.core.datastore.page;
+
+import java.util.concurrent.Callable;
+
+import org.apache.carbondata.core.constants.CarbonCommonConstants;
+import org.apache.carbondata.core.datastore.TableSpec;
+import org.apache.carbondata.core.datastore.compression.CompressorFactory;
+import org.apache.carbondata.core.datastore.page.encoding.EncodedColumnPage;
+import org.apache.carbondata.core.keygenerator.KeyGenerator;
+import org.apache.carbondata.core.keygenerator.factory.KeyGeneratorFactory;
+import org.apache.carbondata.core.localdictionary.generator.LocalDictionaryGenerator;
+import org.apache.carbondata.core.metadata.datatype.DataType;
+import org.apache.carbondata.core.util.CarbonUtil;
+
+public class FallbackDecoderBasedColumnPageEncoder implements Callable<FallbackEncodedColumnPage> {
+ /**
+ * actual local dictionary generated column page
+ */
+ private EncodedColumnPage encodedColumnPage;
+
+ /**
+ * actual index in the page
+ * this is required as in a blocklet few pages will be local dictionary
+ * encoded and few pages will be plain text encoding
+ * in this case local dictionary encoded page
+ */
+ private int pageIndex;
+
+ private LocalDictionaryGenerator localDictionaryGenerator;
+
+ public FallbackDecoderBasedColumnPageEncoder(EncodedColumnPage encodedColumnPage, int pageIndex,
+ LocalDictionaryGenerator localDictionaryGenerator) {
+ this.encodedColumnPage = encodedColumnPage;
+ this.pageIndex = pageIndex;
+ this.localDictionaryGenerator = localDictionaryGenerator;
+ }
+
+ @Override public FallbackEncodedColumnPage call() throws Exception {
+
+ // uncompress the encoded column page
+ byte[] bytes = CompressorFactory.getInstance().getCompressor()
--- End diff --
emm, PR #2628 changed this. We should get the compressor from input configuration or from the metadata.
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/114/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8169/
---
[GitHub] carbondata pull request #2662: [WIP][CARBONDATA-2889]Add decoder based fallb...
Posted by akashrn5 <gi...@git.apache.org>.
Github user akashrn5 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r213922450
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/blocklet/EncodedBlocklet.java ---
@@ -87,19 +91,24 @@ private void addPageMetadata(EncodedTablePage encodedTablePage) {
* @param encodedTablePage
* encoded table page
*/
- private void addEncodedMeasurePage(EncodedTablePage encodedTablePage) {
+ private void addEncodedMeasurePage(EncodedTablePage encodedTablePage,
+ Map<String, LocalDictionaryGenerator> localDictionaryGeneratorMap) {
// for first page create new list
if (null == encodedMeasureColumnPages) {
encodedMeasureColumnPages = new ArrayList<>();
// adding measure pages
for (int i = 0; i < encodedTablePage.getNumMeasures(); i++) {
- BlockletEncodedColumnPage blockletEncodedColumnPage = new BlockletEncodedColumnPage(null);
- blockletEncodedColumnPage.addEncodedColumnColumnPage(encodedTablePage.getMeasure(i));
+ BlockletEncodedColumnPage blockletEncodedColumnPage = new BlockletEncodedColumnPage(null,
+ Boolean.parseBoolean(CarbonProperties.getInstance()
--- End diff --
i will take the performnce report with this change, then we can decide
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/267/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8170/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8332/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/164/
---
[GitHub] carbondata pull request #2662: [CARBONDATA-2889]Add decoder based fallback m...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/2662
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on the issue:
https://github.com/apache/carbondata/pull/2662
@akashrn5 Please publish the performance and memory report
---
[GitHub] carbondata pull request #2662: [WIP][CARBONDATA-2889]Add decoder based fallb...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r214505238
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/FallbackDecoderBasedColumnPageEncoder.java ---
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.carbondata.core.datastore.page;
+
+import java.nio.ByteBuffer;
+import java.util.concurrent.Callable;
+
+import org.apache.carbondata.core.constants.CarbonCommonConstants;
+import org.apache.carbondata.core.datastore.TableSpec;
+import org.apache.carbondata.core.datastore.columnar.UnBlockIndexer;
+import org.apache.carbondata.core.datastore.compression.CompressorFactory;
+import org.apache.carbondata.core.datastore.page.encoding.EncodedColumnPage;
+import org.apache.carbondata.core.keygenerator.KeyGenerator;
+import org.apache.carbondata.core.keygenerator.factory.KeyGeneratorFactory;
+import org.apache.carbondata.core.localdictionary.generator.LocalDictionaryGenerator;
+import org.apache.carbondata.core.metadata.datatype.DataType;
+import org.apache.carbondata.core.util.CarbonUtil;
+import org.apache.carbondata.format.Encoding;
+
+public class FallbackDecoderBasedColumnPageEncoder implements Callable<FallbackEncodedColumnPage> {
--- End diff --
change class name to DecoderBasedFallbackEncoder
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on the issue:
https://github.com/apache/carbondata/pull/2662
retest this please
---
[GitHub] carbondata pull request #2662: [WIP][CARBONDATA-2889]Add decoder based fallb...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r214505217
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/blocklet/BlockletEncodedColumnPage.java ---
@@ -86,7 +92,8 @@
* @param encodedColumnPage
* encoded column page
*/
- void addEncodedColumnColumnPage(EncodedColumnPage encodedColumnPage) {
+ void addEncodedColumnColumnPage(EncodedColumnPage encodedColumnPage,
+ LocalDictionaryGenerator localDictionaryGenerator) {
--- End diff --
better to add local dictionary generator in constructor
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/23/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on the issue:
https://github.com/apache/carbondata/pull/2662
retest this please
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8215/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/144/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.3/8429/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by akashrn5 <gi...@git.apache.org>.
Github user akashrn5 commented on the issue:
https://github.com/apache/carbondata/pull/2662
@kumarvishal09 @jackylk i have updated the PR description with performance and memory report, i have published the result in mail also, please have a look
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/98/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/163/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8249/
---
[GitHub] carbondata pull request #2662: [WIP][CARBONDATA-2889]Add decoder based fallb...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r213160857
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/blocklet/EncodedBlocklet.java ---
@@ -87,19 +91,24 @@ private void addPageMetadata(EncodedTablePage encodedTablePage) {
* @param encodedTablePage
* encoded table page
*/
- private void addEncodedMeasurePage(EncodedTablePage encodedTablePage) {
+ private void addEncodedMeasurePage(EncodedTablePage encodedTablePage,
+ Map<String, LocalDictionaryGenerator> localDictionaryGeneratorMap) {
// for first page create new list
if (null == encodedMeasureColumnPages) {
encodedMeasureColumnPages = new ArrayList<>();
// adding measure pages
for (int i = 0; i < encodedTablePage.getNumMeasures(); i++) {
- BlockletEncodedColumnPage blockletEncodedColumnPage = new BlockletEncodedColumnPage(null);
- blockletEncodedColumnPage.addEncodedColumnColumnPage(encodedTablePage.getMeasure(i));
+ BlockletEncodedColumnPage blockletEncodedColumnPage = new BlockletEncodedColumnPage(null,
+ Boolean.parseBoolean(CarbonProperties.getInstance()
--- End diff --
What if the configuration is changed during data loading? So that each column page will have different configuration, will this be OK?
---
[GitHub] carbondata pull request #2662: [WIP][CARBONDATA-2889]Add decoder based fallb...
Posted by akashrn5 <gi...@git.apache.org>.
Github user akashrn5 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r214426930
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/blocklet/EncodedBlocklet.java ---
@@ -87,19 +91,24 @@ private void addPageMetadata(EncodedTablePage encodedTablePage) {
* @param encodedTablePage
* encoded table page
*/
- private void addEncodedMeasurePage(EncodedTablePage encodedTablePage) {
+ private void addEncodedMeasurePage(EncodedTablePage encodedTablePage,
+ Map<String, LocalDictionaryGenerator> localDictionaryGeneratorMap) {
// for first page create new list
if (null == encodedMeasureColumnPages) {
encodedMeasureColumnPages = new ArrayList<>();
// adding measure pages
for (int i = 0; i < encodedTablePage.getNumMeasures(); i++) {
- BlockletEncodedColumnPage blockletEncodedColumnPage = new BlockletEncodedColumnPage(null);
- blockletEncodedColumnPage.addEncodedColumnColumnPage(encodedTablePage.getMeasure(i));
+ BlockletEncodedColumnPage blockletEncodedColumnPage = new BlockletEncodedColumnPage(null,
+ Boolean.parseBoolean(CarbonProperties.getInstance()
--- End diff --
@xuchuanyin i have tested and results i have published, i think we can keep it as property and make default by true, as we getting good result with respect to memory
---
[GitHub] carbondata pull request #2662: [CARBONDATA-2889]Add decoder based fallback m...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r214545340
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/blocklet/EncodedBlocklet.java ---
@@ -110,21 +120,24 @@ private void addEncodedMeasurePage(EncodedTablePage encodedTablePage) {
* @param encodedTablePage
* encoded table page
*/
- private void addEncodedDimensionPage(EncodedTablePage encodedTablePage) {
+ private void addEncodedDimensionPage(EncodedTablePage encodedTablePage,
+ Map<String, LocalDictionaryGenerator> localDictionaryGeneratorMap) {
--- End diff --
Local dictionary map instance will not change and EncodedBlocklet instance is created only once, better to pass in constructor
---
[GitHub] carbondata pull request #2662: [WIP][CARBONDATA-2889]Add decoder based fallb...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r214505230
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/FallbackActualDataBasedColumnPageEncoder.java ---
@@ -19,17 +19,17 @@
import java.util.concurrent.Callable;
import org.apache.carbondata.core.datastore.TableSpec;
-import org.apache.carbondata.core.datastore.page.encoding.ColumnPageEncoder;
-import org.apache.carbondata.core.datastore.page.encoding.DefaultEncodingFactory;
import org.apache.carbondata.core.datastore.page.encoding.EncodedColumnPage;
+import org.apache.carbondata.core.util.CarbonUtil;
/**
* Below class will be used to encode column pages for which local dictionary was generated
* but all the pages in blocklet was not encoded with local dictionary.
* This is required as all the pages of a column in blocklet either it will be local dictionary
* encoded or without local dictionary encoded.
*/
-public class FallbackColumnPageEncoder implements Callable<FallbackEncodedColumnPage> {
+public class FallbackActualDataBasedColumnPageEncoder
--- End diff --
Change class name to ActualDataBasedFallbackEncoder
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/189/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/99/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/81/
---
[GitHub] carbondata pull request #2662: [WIP][CARBONDATA-2889]Add decoder based fallb...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r214504736
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/blocklet/BlockletEncodedColumnPage.java ---
@@ -86,7 +92,8 @@
* @param encodedColumnPage
* encoded column page
*/
- void addEncodedColumnColumnPage(EncodedColumnPage encodedColumnPage) {
+ void addEncodedColumnColumnPage(EncodedColumnPage encodedColumnPage,
--- End diff --
Can u Please update the method name addEncodedColumnPage.
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2662
Please fill PR description and do you have memory consumption comparison for this PR? @akashrn5
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8220/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/282/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by akashrn5 <gi...@git.apache.org>.
Github user akashrn5 commented on the issue:
https://github.com/apache/carbondata/pull/2662
@jackylk i will test and publish the comparision here, i am still testing this PR
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/116/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2662
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6420/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by akashrn5 <gi...@git.apache.org>.
Github user akashrn5 commented on the issue:
https://github.com/apache/carbondata/pull/2662
@kumarvishal09 handled comments, please review
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8152/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/98/
---
[GitHub] carbondata pull request #2662: [WIP][CARBONDATA-2889]Add decoder based fallb...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r214504750
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/blocklet/BlockletEncodedColumnPage.java ---
@@ -105,8 +112,15 @@ void addEncodedColumnColumnPage(EncodedColumnPage encodedColumnPage) {
LOGGER.info(
"Local dictionary Fallback is initiated for column: " + this.columnName + " for page:"
+ encodedColumnPageList.size());
- fallbackFutureQueue.add(fallbackExecutorService
- .submit(new FallbackColumnPageEncoder(encodedColumnPage, encodedColumnPageList.size())));
+ if (isDecoderBasedFallBackEnabled) {
--- End diff --
Move this code to some private metod and pass encodedColumnPage and pageIndex
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/178/
---
[GitHub] carbondata pull request #2662: [WIP][CARBONDATA-2889]Add decoder based fallb...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2662#discussion_r214504804
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/blocklet/EncodedBlocklet.java ---
@@ -87,19 +91,24 @@ private void addPageMetadata(EncodedTablePage encodedTablePage) {
* @param encodedTablePage
* encoded table page
*/
- private void addEncodedMeasurePage(EncodedTablePage encodedTablePage) {
+ private void addEncodedMeasurePage(EncodedTablePage encodedTablePage,
+ Map<String, LocalDictionaryGenerator> localDictionaryGeneratorMap) {
// for first page create new list
if (null == encodedMeasureColumnPages) {
encodedMeasureColumnPages = new ArrayList<>();
// adding measure pages
for (int i = 0; i < encodedTablePage.getNumMeasures(); i++) {
- BlockletEncodedColumnPage blockletEncodedColumnPage = new BlockletEncodedColumnPage(null);
- blockletEncodedColumnPage.addEncodedColumnColumnPage(encodedTablePage.getMeasure(i));
+ BlockletEncodedColumnPage blockletEncodedColumnPage = new BlockletEncodedColumnPage(null,
+ Boolean.parseBoolean(CarbonProperties.getInstance()
--- End diff --
Instead of parsing every time for each encodedPage parse once in constructor and add in private field ...for measure u can directly pass false as local dictionary will not generated for measure page
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2662
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6464/
---
[GitHub] carbondata issue #2662: [WIP][CARBONDATA-2889]Add decoder based fallback mec...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8187/
---
[GitHub] carbondata issue #2662: [CARBONDATA-2889]Add decoder based fallback mechanis...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2662
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8234/
---