[GitHub] carbondata pull request #2907: [CARBONDATA-3031] refining usage of numberofc...

[qiuchenjian-2]

GitHub user Sssan520 opened a pull request:

    https://github.com/apache/carbondata/pull/2907

    [CARBONDATA-3031] refining usage of numberofcores in CarbonProperties

    **1**. many places use the function 'getNumOfCores' of CarbonProperties which returns **the loading cores.**
    **2**. so if we still use the value in scene like 'query' or 'compaction' , it will be confused.
   
    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/Sssan520/carbondata numcores

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/2907.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 #2907
   
----
commit f8e7cdfecaed9079b8a90b2604a89ec9630a2d90
Author: Sssan520 <liangap2008@...>
Date:   2018-07-02T11:12:24Z

    [CARBONDATA-3031] refining usage of numberofcores in CarbonProperties

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2907
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1327/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2907#discussion_r231777987
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java ---
    @@ -951,27 +951,46 @@ public void print() {
         return compactionSize;
       }
     
    -  /**
    -   * Number of cores should be used while loading data.
    -   *
    -   * @return
    -   */
    -  public int getNumberOfCores() {
    +  public int getNumberOfCores(String key) {
    --- End diff --
   
    I think this method can be private now


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2907#discussion_r231778046
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java ---
    @@ -951,27 +951,46 @@ public void print() {
         return compactionSize;
       }
     
    -  /**
    -   * Number of cores should be used while loading data.
    -   *
    -   * @return
    -   */
    -  public int getNumberOfCores() {
    +  public int getNumberOfCores(String key) {
         int numberOfCores;
         try {
           numberOfCores = Integer.parseInt(
               CarbonProperties.getInstance().getProperty(
    -              CarbonCommonConstants.NUM_CORES_LOADING,
    +              key,
                   CarbonCommonConstants.NUM_CORES_DEFAULT_VAL));
         } catch (NumberFormatException exc) {
    -      LOGGER.warn("Configured value for property " + CarbonCommonConstants.NUM_CORES_LOADING
    +      LOGGER.warn("Configured value for property " + key
               + " is wrong. Falling back to the default value "
               + CarbonCommonConstants.NUM_CORES_DEFAULT_VAL);
           numberOfCores = Integer.parseInt(CarbonCommonConstants.NUM_CORES_DEFAULT_VAL);
         }
         return numberOfCores;
       }
     
    +  /**
    +   * Number of cores should be used while loading data.
    +   * @return
    --- End diff --
   
    we can just remove the @return for the following three methods


---

[qiuchenjian-2]

Github user Sssan520 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2907
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1337/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2907#discussion_r232151408
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java ---
    @@ -951,27 +951,46 @@ public void print() {
         return compactionSize;
       }
     
    -  /**
    -   * Number of cores should be used while loading data.
    -   *
    -   * @return
    -   */
    -  public int getNumberOfCores() {
    +  public int getNumberOfCores(String key) {
         int numberOfCores;
         try {
           numberOfCores = Integer.parseInt(
               CarbonProperties.getInstance().getProperty(
    -              CarbonCommonConstants.NUM_CORES_LOADING,
    +              key,
                   CarbonCommonConstants.NUM_CORES_DEFAULT_VAL));
         } catch (NumberFormatException exc) {
    -      LOGGER.warn("Configured value for property " + CarbonCommonConstants.NUM_CORES_LOADING
    +      LOGGER.warn("Configured value for property " + key
               + " is wrong. Falling back to the default value "
               + CarbonCommonConstants.NUM_CORES_DEFAULT_VAL);
           numberOfCores = Integer.parseInt(CarbonCommonConstants.NUM_CORES_DEFAULT_VAL);
         }
         return numberOfCores;
       }
     
    +  /**
    +   * Number of cores should be used while loading data.
    +   * @return
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2907#discussion_r232151415
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java ---
    @@ -951,27 +951,46 @@ public void print() {
         return compactionSize;
       }
     
    -  /**
    -   * Number of cores should be used while loading data.
    -   *
    -   * @return
    -   */
    -  public int getNumberOfCores() {
    +  public int getNumberOfCores(String key) {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user Sssan520 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2907
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1343/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2907
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1344/



---

[qiuchenjian-2]

Github user Sssan520 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---