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

[asfgit]

Github user asfgit closed the pull request at:

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


---

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

[Sssan520]

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


---

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

[Sssan520]

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


---

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

[xuchuanyin]

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


---

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

[xuchuanyin]

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


---

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

[Sssan520]

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 
Date:   2018-07-02T11:12:24Z

[CARBONDATA-3031] refining usage of numberofcores in CarbonProperties




---