[GitHub] carbondata pull request #2875: [CARBONDATA-3038] Refactor dynamic configurat...

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2875#discussion_r230629669
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -117,77 +126,106 @@ private CarbonCommonConstants() {
        * CARBON_TIMESTAMP
        */
       public static final String CARBON_TIMESTAMP_MILLIS = "dd-MM-yyyy HH:mm:ss:SSS";
    +
       /**
    -   * compressor for writing/reading carbondata file
    +   * Property for specifying the format of DATE data type column.
    +   * e.g. yyyy/MM/dd , or using default value
        */
    +  @CarbonProperty
    +  public static final String CARBON_DATE_FORMAT = "carbon.date.format";
    +
    +  /**
    +   * default value
    +   */
    +  public static final String CARBON_DATE_DEFAULT_FORMAT = "yyyy-MM-dd";
    +
    +  /**
    +   * compressor for writing/reading CarbonData file
    +   */
    +  @CarbonProperty
       public static final String COMPRESSOR = "carbon.column.compressor";
     
       /**
        * default compressor is snappy
        */
       public static final String DEFAULT_COMPRESSOR = "snappy";
    +
       /**
    -   * ZOOKEEPER_ENABLE_LOCK if this is set to true then zookeeper will be used to handle locking
    +   * ZOOKEEPER_ENABLE_LOCK if this is set to true then zookeeper
    +   * will be used to handle locking
        * mechanism of carbon
        */
       @CarbonProperty
       public static final String LOCK_TYPE = "carbon.lock.type";
     
    +  /**
    +   * ZOOKEEPER_ENABLE_DEFAULT the default value for zookeeper will be true for carbon
    +   */
    +  public static final String LOCK_TYPE_DEFAULT = "LOCALLOCK";
    +
       /**
        * Specifies the path where the lock files have to be created.
        * By default, lock files are created in table path.
        */
       @CarbonProperty
       public static final String LOCK_PATH = "carbon.lock.path";
     
    -  /**
    -   * ZOOKEEPER_ENABLE_DEFAULT the default value for zookeeper will be true for carbon
    -   */
    -  public static final String LOCK_TYPE_DEFAULT = "LOCALLOCK";
    +  public static final String LOCK_PATH_DEFAULT = "";
     
       /**
        * ZOOKEEPER_LOCATION this is the location in zookeeper file system where locks are created.
        * mechanism of carbon
        */
       public static final String ZOOKEEPER_LOCATION = "/CarbonLocks";
    +
       /**
    -   * xxhash algorithm property for hashmap
    +   * xxhash algorithm property for hashMap
        */
    +  @CarbonProperty
       public static final String ENABLE_XXHASH = "carbon.enableXXHash";
     
       /**
    -   * xxhash algorithm property for hashmap. Default value false
    +   * xxhash algorithm property for hashMap Default value false
    --- End diff --
   
    ok, keep


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2875#discussion_r230629760
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -117,77 +126,106 @@ private CarbonCommonConstants() {
        * CARBON_TIMESTAMP
        */
       public static final String CARBON_TIMESTAMP_MILLIS = "dd-MM-yyyy HH:mm:ss:SSS";
    +
       /**
    -   * compressor for writing/reading carbondata file
    +   * Property for specifying the format of DATE data type column.
    +   * e.g. yyyy/MM/dd , or using default value
        */
    +  @CarbonProperty
    +  public static final String CARBON_DATE_FORMAT = "carbon.date.format";
    +
    +  /**
    +   * default value
    +   */
    +  public static final String CARBON_DATE_DEFAULT_FORMAT = "yyyy-MM-dd";
    +
    +  /**
    +   * compressor for writing/reading CarbonData file
    +   */
    +  @CarbonProperty
       public static final String COMPRESSOR = "carbon.column.compressor";
     
       /**
        * default compressor is snappy
        */
       public static final String DEFAULT_COMPRESSOR = "snappy";
    +
       /**
    -   * ZOOKEEPER_ENABLE_LOCK if this is set to true then zookeeper will be used to handle locking
    +   * ZOOKEEPER_ENABLE_LOCK if this is set to true then zookeeper
    +   * will be used to handle locking
        * mechanism of carbon
        */
       @CarbonProperty
       public static final String LOCK_TYPE = "carbon.lock.type";
     
    +  /**
    +   * ZOOKEEPER_ENABLE_DEFAULT the default value for zookeeper will be true for carbon
    +   */
    +  public static final String LOCK_TYPE_DEFAULT = "LOCALLOCK";
    +
       /**
        * Specifies the path where the lock files have to be created.
        * By default, lock files are created in table path.
        */
       @CarbonProperty
       public static final String LOCK_PATH = "carbon.lock.path";
     
    -  /**
    -   * ZOOKEEPER_ENABLE_DEFAULT the default value for zookeeper will be true for carbon
    -   */
    -  public static final String LOCK_TYPE_DEFAULT = "LOCALLOCK";
    +  public static final String LOCK_PATH_DEFAULT = "";
     
       /**
        * ZOOKEEPER_LOCATION this is the location in zookeeper file system where locks are created.
        * mechanism of carbon
        */
       public static final String ZOOKEEPER_LOCATION = "/CarbonLocks";
    +
       /**
    -   * xxhash algorithm property for hashmap
    +   * xxhash algorithm property for hashMap
        */
    +  @CarbonProperty
       public static final String ENABLE_XXHASH = "carbon.enableXXHash";
     
       /**
    -   * xxhash algorithm property for hashmap. Default value false
    +   * xxhash algorithm property for hashMap Default value false
        */
       public static final String ENABLE_XXHASH_DEFAULT = "true";
    +
       /**
        * System property to enable or disable local dictionary generation
        */
    +  @CarbonProperty
       public static final String LOCAL_DICTIONARY_SYSTEM_ENABLE = "carbon.local.dictionary.enable";
     
       /**
        * System property to enable or disable decoder based local dictionary fallback
        */
    +  @CarbonProperty
       public static final String LOCAL_DICTIONARY_DECODER_BASED_FALLBACK =
           "carbon.local.dictionary.decoder.fallback";
     
       /**
        * System property to enable or disable decoder based local dictionary fallback default value
        */
       public static final String LOCAL_DICTIONARY_DECODER_BASED_FALLBACK_DEFAULT = "true";
    +
       /**
        * zookeeper url key
        */
    +  @CarbonProperty
       public static final String ZOOKEEPER_URL = "spark.deploy.zookeeper.url";
    +
       /**
        * carbon data file version property
        */
    +  @CarbonProperty
       public static final String CARBON_DATA_FILE_VERSION = "carbon.data.file.version";
    +
       @CarbonProperty
       public static final String SPARK_SCHEMA_STRING_LENGTH_THRESHOLD =
           "spark.sql.sources.schemaStringLengthThreshold";
     
       public static final int SPARK_SCHEMA_STRING_LENGTH_THRESHOLD_DEFAULT = 4000;
     
    +  //TODO: add annotation and whether change to dynamic
    --- End diff --
   
    ok, done


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2875#discussion_r230629785
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -435,53 +520,77 @@ private CarbonCommonConstants() {
       public static final int SORT_INTERMEDIATE_FILES_LIMIT_MIN = 2;
     
       public static final int SORT_INTERMEDIATE_FILES_LIMIT_MAX = 50;
    +
       /**
        * SORT_FILE_WRITE_BUFFER_SIZE
        */
       @CarbonProperty
       public static final String CARBON_SORT_FILE_WRITE_BUFFER_SIZE =
           "carbon.sort.file.write.buffer.size";
    -  public static final int CARBON_SORT_FILE_WRITE_BUFFER_SIZE_MIN = 10240;
    -  public static final int CARBON_SORT_FILE_WRITE_BUFFER_SIZE_MAX = 10485760;
    +
       /**
        * SORT_FILE_WRITE_BUFFER_SIZE_DEFAULT_VALUE
        */
       public static final String CARBON_SORT_FILE_WRITE_BUFFER_SIZE_DEFAULT_VALUE = "16384";
    +
    +  public static final int CARBON_SORT_FILE_WRITE_BUFFER_SIZE_MIN = 10240;
    +
    +  public static final int CARBON_SORT_FILE_WRITE_BUFFER_SIZE_MAX = 10485760;
    +
       /**
        * CSV_READ_BUFFER_SIZE
        */
       @CarbonProperty
       public static final String CSV_READ_BUFFER_SIZE = "carbon.csv.read.buffersize.byte";
    +
       /**
        * CSV_READ_BUFFER_SIZE
    +   * default value is 1mb
        */
    -  public static final String CSV_READ_BUFFER_SIZE_DEFAULT = "1048576"; // 1mb
    +  public static final String CSV_READ_BUFFER_SIZE_DEFAULT = "1048576";
    +
       /**
    -   * min value for csv read buffer size
    +   * min value for csv read buffer size, 10 kb
        */
    -  public static final int CSV_READ_BUFFER_SIZE_MIN = 10240; //10 kb
    +  public static final int CSV_READ_BUFFER_SIZE_MIN = 10240;
    +
       /**
    -   * max value for csv read buffer size
    +   * max value for csv read buffer size, 10 mb
        */
    -  public static final int CSV_READ_BUFFER_SIZE_MAX = 10485760; // 10 mb
    +  public static final int CSV_READ_BUFFER_SIZE_MAX = 10485760;
    +
       /**
        * CARBON_MERGE_SORT_READER_THREAD
        */
       @CarbonProperty
       public static final String CARBON_MERGE_SORT_READER_THREAD = "carbon.merge.sort.reader.thread";
    +
       /**
    -   * CARBON_MERGE_SORT_READER_THREAD_DEFAULTVALUE
    +   * CARBON_MERGE_SORT_READER_THREAD DEFAULT value
        */
       public static final String CARBON_MERGE_SORT_READER_THREAD_DEFAULTVALUE = "3";
    +
       /**
        * TIME_STAT_UTIL_TYPE
    +   * TODO:dynamic
    --- End diff --
   
    removed, this property should can be dynamic configurable


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2875#discussion_r230629792
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -435,53 +520,77 @@ private CarbonCommonConstants() {
       public static final int SORT_INTERMEDIATE_FILES_LIMIT_MIN = 2;
     
       public static final int SORT_INTERMEDIATE_FILES_LIMIT_MAX = 50;
    +
       /**
        * SORT_FILE_WRITE_BUFFER_SIZE
        */
       @CarbonProperty
       public static final String CARBON_SORT_FILE_WRITE_BUFFER_SIZE =
           "carbon.sort.file.write.buffer.size";
    -  public static final int CARBON_SORT_FILE_WRITE_BUFFER_SIZE_MIN = 10240;
    -  public static final int CARBON_SORT_FILE_WRITE_BUFFER_SIZE_MAX = 10485760;
    +
       /**
        * SORT_FILE_WRITE_BUFFER_SIZE_DEFAULT_VALUE
        */
       public static final String CARBON_SORT_FILE_WRITE_BUFFER_SIZE_DEFAULT_VALUE = "16384";
    +
    +  public static final int CARBON_SORT_FILE_WRITE_BUFFER_SIZE_MIN = 10240;
    +
    +  public static final int CARBON_SORT_FILE_WRITE_BUFFER_SIZE_MAX = 10485760;
    +
       /**
        * CSV_READ_BUFFER_SIZE
        */
       @CarbonProperty
       public static final String CSV_READ_BUFFER_SIZE = "carbon.csv.read.buffersize.byte";
    +
       /**
        * CSV_READ_BUFFER_SIZE
    +   * default value is 1mb
        */
    -  public static final String CSV_READ_BUFFER_SIZE_DEFAULT = "1048576"; // 1mb
    +  public static final String CSV_READ_BUFFER_SIZE_DEFAULT = "1048576";
    +
       /**
    -   * min value for csv read buffer size
    +   * min value for csv read buffer size, 10 kb
        */
    -  public static final int CSV_READ_BUFFER_SIZE_MIN = 10240; //10 kb
    +  public static final int CSV_READ_BUFFER_SIZE_MIN = 10240;
    +
       /**
    -   * max value for csv read buffer size
    +   * max value for csv read buffer size, 10 mb
        */
    -  public static final int CSV_READ_BUFFER_SIZE_MAX = 10485760; // 10 mb
    +  public static final int CSV_READ_BUFFER_SIZE_MAX = 10485760;
    +
       /**
        * CARBON_MERGE_SORT_READER_THREAD
        */
       @CarbonProperty
       public static final String CARBON_MERGE_SORT_READER_THREAD = "carbon.merge.sort.reader.thread";
    +
       /**
    -   * CARBON_MERGE_SORT_READER_THREAD_DEFAULTVALUE
    +   * CARBON_MERGE_SORT_READER_THREAD DEFAULT value
        */
       public static final String CARBON_MERGE_SORT_READER_THREAD_DEFAULTVALUE = "3";
    +
       /**
        * TIME_STAT_UTIL_TYPE
    +   * TODO:dynamic
        */
       @CarbonProperty
       public static final String ENABLE_DATA_LOADING_STATISTICS = "enable.data.loading.statistics";
    +
       /**
        * TIME_STAT_UTIL_TYPE_DEFAULT
        */
    +
       public static final String ENABLE_DATA_LOADING_STATISTICS_DEFAULT = "false";
    +
    +  /**
    +   * NUMBER_OF_TRIES_FOR_LOAD_METADATA_LOCK
    +   * TODO: dynamic
    --- End diff --
   
    removed, this property should can be dynamic configurable


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2875#discussion_r230629800
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -594,6 +713,7 @@ private CarbonCommonConstants() {
     
       /**
        * This property will determine the loads of how many days can be compacted.
    +   * TODO: dynamic
    --- End diff --
   
    removed, this property should can be dynamic configurable


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2875#discussion_r230629803
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -897,7 +1041,8 @@ private CarbonCommonConstants() {
       /**
        * property for configuring parallelism per segment when doing an update. Increase this
        * value will avoid data screw problem for a large segment.
    -   * Refer to CARBONDATA-1373 for more details.
    +   * Refer to CarbonData-1373 for more details.
    +   * TODO: dynamic
    --- End diff --
   
    removed, this property should can be dynamic configurable


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2875#discussion_r230629905
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -975,16 +1140,39 @@ private CarbonCommonConstants() {
        * make it less and not the index writing speed.
        */
       public static final String CARBON_LUCENE_COMPRESSION_MODE_DEFAULT = "speed";
    +
       /**
        * The node loads the smallest amount of data
        */
       @CarbonProperty
       public static final String CARBON_LOAD_MIN_SIZE_INMB = "load_min_size_inmb";
    +
    +  /**
    +   * the node minimum load data default value
    +   */
       public static final String CARBON_LOAD_MIN_SIZE_INMB_DEFAULT = "0";
    +
    +  @CarbonProperty
    +  public static final String IN_MEMORY_FOR_SORT_DATA_IN_MB = "sort.inmemory.size.inmb";
    +
    +  public static final String IN_MEMORY_FOR_SORT_DATA_IN_MB_DEFAULT = "1024";
    +
    +  /**
    +   * carbon sort size
    +   */
    +  @Deprecated
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2875#discussion_r230630756
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1744,89 +2021,101 @@ private CarbonCommonConstants() {
        * BYTE_ENCODING
        */
       public static final String BYTE_ENCODING = "ISO-8859-1";
    +
    --- End diff --
   
    move it to Query parameter, we can support dynamic=true for it in a new PR. There are others parameters like that.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1259/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1474/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9523/



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    @jackylk  CI pass, please check it.


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2875#discussion_r230991320
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1744,89 +2021,101 @@ private CarbonCommonConstants() {
        * BYTE_ENCODING
        */
       public static final String BYTE_ENCODING = "ISO-8859-1";
    +
    --- End diff --
   
    support configure carbon.push.rowfilters.for.vector in set command, dynamic configurable in this PR


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    test it by “Inserting table: support boolean and other data type, big file” test case in org.apache.carbondata.spark.testsuite.booleantype.BooleanDataTypesBigFileTest. when run "insert into boolean_table2 select * from boolean_table", it will match org.apache.spark.sql.execution.strategy.CarbonLateDecodeStrategy#pruneFilterProjectRaw


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1296/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1510/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9557/



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    retest thi please


---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2875
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1522/



---