[CARBONDATA-1305] Add limit for external dictionary values #1166

kunal642 · 2017-07-12T15:28:23Z

Analysis: During dictionary creation the dictionary values are kept in a HashSet. When the size of hashset reaches more than 500000000 this exception is thrown.

Solution: Limit the dictionary values to 10000000

asfgit · 2017-07-12T15:28:25Z

Can one of the admins verify this patch?

asfgit · 2017-07-12T15:28:25Z

Can one of the admins verify this patch?

CarbonDataQA · 2017-07-12T15:29:59Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/462/

CarbonDataQA · 2017-07-12T15:30:59Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3050/

CarbonDataQA · 2017-07-12T15:51:17Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3051/

CarbonDataQA · 2017-07-12T16:01:06Z

Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/463/

manishgupta88 · 2017-07-13T05:29:18Z

core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java

+  /**
+   *  Max number of dictionary values that can be given with external dictionary
+   */
+  public static final int MAX_EXTERNAL_DICTIONARY_SIZE = 10000000;


Move this constant to specific file usage as this is not configurable

manishgupta88 · 2017-07-13T05:31:10Z

.../spark-common/src/main/scala/org/apache/carbondata/spark/rdd/CarbonGlobalDictionaryRDD.scala

@@ -83,7 +84,12 @@ case class PrimitiveParser(dimension: CarbonDimension,

  def parseString(input: String): Unit = {
    if (hasDictEncoding && input != null) {
-      set.add(input)
+      if (set.size < CarbonCommonConstants.MAX_EXTERNAL_DICTIONARY_SIZE) {


This check will be done for each value for each column...please ensure there is no performance degradation in dictionary generation due to this check...try with some 100 columns with and without this change

CarbonDataQA · 2017-07-13T05:59:05Z

Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/471/

CarbonDataQA · 2017-07-13T05:59:11Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3060/

gvramana · 2017-07-14T06:52:11Z

...ava/org/apache/carbondata/processing/newflow/exception/DictionaryLimitExceededException.java

+
+package org.apache.carbondata.processing.newflow.exception;
+
+public class DictionaryLimitExceededException extends CarbonDataLoadingException {


Use no retry exception instead of specific exception.

gvramana · 2017-07-14T07:11:29Z

.../spark-common/src/main/scala/org/apache/carbondata/spark/rdd/CarbonGlobalDictionaryRDD.scala

@@ -460,6 +467,10 @@ class CarbonGlobalDictionaryGenerateRDD(
                      s"\n write sort info: $sortIndexWriteTime")
        }
      } catch {
+        case dictionaryException: DictionaryLimitExceededException =>


use a generic NoRetryExceptionClass instead of Specific exception class.

CarbonDataQA · 2017-07-14T07:29:00Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3078/

CarbonDataQA · 2017-07-14T07:30:13Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/489/

CarbonDataQA · 2017-07-14T08:13:40Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/490/

CarbonDataQA · 2017-07-14T08:15:25Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3079/

CarbonDataQA · 2017-07-14T09:47:25Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3081/

CarbonDataQA · 2017-07-14T09:48:34Z

Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/492/

CarbonDataQA · 2017-07-24T14:28:15Z

Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/590/

CarbonDataQA · 2017-07-24T14:28:58Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3185/

CarbonDataQA · 2017-07-24T14:37:44Z

Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/591/

CarbonDataQA · 2017-07-24T14:40:32Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3186/

CarbonDataQA · 2017-08-02T08:45:39Z

Can one of the admins verify this patch?

CarbonDataQA · 2017-08-30T14:42:53Z

SDV Build Success with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/327/

CarbonDataQA · 2017-08-31T00:52:00Z

SDV Build Success with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/336/

gvramana · 2017-08-31T14:52:02Z

retest this please

kunal642 · 2017-09-03T09:30:55Z

retest this please

ravipesala · 2017-09-03T09:58:15Z

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/489/

gvramana · 2017-09-04T06:02:39Z

.../spark-common/src/main/scala/org/apache/carbondata/spark/rdd/CarbonGlobalDictionaryRDD.scala

+      if (set.size < CarbonLoadOptionConstants.MAX_EXTERNAL_DICTIONARY_SIZE) {
+        set.add(input)
+      } else {
+        throw new NoRetryException(s"Cannot provide more than ${


Also handle NoRetryException for update also inset in case of badrecords

gvramana · 2017-09-04T06:22:44Z

retest this please

ravipesala · 2017-09-04T08:05:58Z

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/501/

kunal642 · 2017-09-04T10:57:20Z

retest this please

ravipesala · 2017-09-04T13:33:34Z

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/508/

gvramana · 2017-09-04T14:32:04Z

SDV one failure is common across other PRs

gvramana · 2017-09-04T14:32:11Z

LGTM

Analysis: During dictionary creation the dictionary values are kept in a HashSet. When the size of hashset reaches more than 500000000 this exception is thrown. Solution: Limit the dictionary values to 10000000 This closes apache#1166

kunal642 changed the title ~~[CARBONDATA-1305] Added limit for external dictionary values~~ [CARBONDATA-1305] Add limit for external dictionary values Jul 12, 2017

kunal642 force-pushed the dictionary_limit branch from d3768a8 to 5f90235 Compare July 12, 2017 15:39

manishgupta88 reviewed Jul 13, 2017

View reviewed changes

kunal642 force-pushed the dictionary_limit branch from 5f90235 to ecb490d Compare July 13, 2017 05:47

gvramana reviewed Jul 14, 2017

View reviewed changes

kunal642 force-pushed the dictionary_limit branch from ecb490d to fd4bec9 Compare July 14, 2017 07:27

kunal642 force-pushed the dictionary_limit branch from fd4bec9 to fe3c28f Compare July 14, 2017 08:03

kunal642 force-pushed the dictionary_limit branch from fe3c28f to 5ab9eff Compare July 14, 2017 09:34

kunal642 force-pushed the dictionary_limit branch 2 times, most recently from 15d32db to 1dab411 Compare July 24, 2017 14:19

kunal642 force-pushed the dictionary_limit branch from 1dab411 to 49f808f Compare August 30, 2017 06:31

kunal642 force-pushed the dictionary_limit branch from 49f808f to b47ba2b Compare August 30, 2017 20:18

added limit for external dictionary values

9a68491

kunal642 force-pushed the dictionary_limit branch from b47ba2b to 9a68491 Compare August 31, 2017 09:21

gvramana approved these changes Sep 4, 2017

View reviewed changes

asfgit closed this in 2e04c35 Sep 4, 2017

kunal642 deleted the dictionary_limit branch October 6, 2017 05:54


		package org.apache.carbondata.processing.newflow.exception;

		public class DictionaryLimitExceededException extends CarbonDataLoadingException {

[CARBONDATA-1305] Add limit for external dictionary values #1166

[CARBONDATA-1305] Add limit for external dictionary values #1166

Conversation

kunal642 commented Jul 12, 2017

asfgit commented Jul 12, 2017

asfgit commented Jul 12, 2017

CarbonDataQA commented Jul 12, 2017

CarbonDataQA commented Jul 12, 2017

CarbonDataQA commented Jul 12, 2017

CarbonDataQA commented Jul 12, 2017

manishgupta88 Jul 13, 2017

Choose a reason for hiding this comment

manishgupta88 Jul 13, 2017

Choose a reason for hiding this comment

CarbonDataQA commented Jul 13, 2017

CarbonDataQA commented Jul 13, 2017

gvramana Jul 14, 2017

Choose a reason for hiding this comment

gvramana Jul 14, 2017

Choose a reason for hiding this comment

CarbonDataQA commented Jul 14, 2017

CarbonDataQA commented Jul 14, 2017

CarbonDataQA commented Jul 14, 2017

CarbonDataQA commented Jul 14, 2017

CarbonDataQA commented Jul 14, 2017

CarbonDataQA commented Jul 14, 2017

CarbonDataQA commented Jul 24, 2017

CarbonDataQA commented Jul 24, 2017

CarbonDataQA commented Jul 24, 2017

CarbonDataQA commented Jul 24, 2017

CarbonDataQA commented Aug 2, 2017

CarbonDataQA commented Aug 30, 2017

CarbonDataQA commented Aug 31, 2017

gvramana commented Aug 31, 2017

kunal642 commented Sep 3, 2017

ravipesala commented Sep 3, 2017

gvramana Sep 4, 2017

Choose a reason for hiding this comment

gvramana commented Sep 4, 2017

ravipesala commented Sep 4, 2017

kunal642 commented Sep 4, 2017

ravipesala commented Sep 4, 2017

gvramana commented Sep 4, 2017

gvramana commented Sep 4, 2017