[GitHub] carbondata pull request #1679: [CARBONDATA-1907] Avoid unnecessary logging t...

[qiuchenjian-2]

GitHub user manishgupta88 opened a pull request:

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

    [CARBONDATA-1907] Avoid unnecessary logging to improve query performance for no dictionary non string columns

    Changes done to return null in case of no dictionary column for non string data types when data is empty. This is done to avoid excessive logging which is impacting the query performance.
   
     - [ ] Any interfaces changed?
     No
     - [ ] Any backward compatibility impacted?
     No
     - [ ] Document update required?
    No
     - [ ] Testing done
    Added UT
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    NA


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/manishgupta88/carbondata empty_byte_fix

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

    https://github.com/apache/carbondata/pull/1679.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 #1679
   
----
commit f5690380ec6d0abaee89acd0bb8fda23eb7f3a33
Author: manishgupta88 <[hidden email]>
Date:   2017-12-19T11:57:30Z

    Changes done to return null in case of no dictionary column for non string data types when data is empty. This is done to avoid excessive logging which is impacting the query performance.

----


---

[qiuchenjian-2]

Github user manishgupta88 commented on the issue:

    https://github.com/apache/carbondata/pull/1679
 
    @kumarvishal09 ..please review


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1679#discussion_r157746071
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/DataTypeUtil.java ---
    @@ -353,12 +353,24 @@ public static Object getDataBasedOnDataTypeForNoDictionaryColumn(byte[] dataInBy
           } else if (actualDataType == DataTypes.STRING) {
             return getDataTypeConverter().convertFromByteToUTF8String(dataInBytes);
           } else if (actualDataType == DataTypes.SHORT) {
    +        if (isEmptyByteArray(dataInBytes)) {
    --- End diff --
   
    Please add a comment .. for non string type no dictionary column empty byte array is null value so no  need to parse


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1679#discussion_r157746485
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/AbstractQueryExecutor.java ---
    @@ -544,12 +543,7 @@ private int getKeySize(List<QueryDimension> queryDimension,
         }
         UnsafeMemoryManager.INSTANCE.freeMemoryAll(ThreadLocalTaskInfo.getCarbonTaskInfo().getTaskId());
         if (null != queryProperties.executorService) {
    -      queryProperties.executorService.shutdown();
    -      try {
    -        queryProperties.executorService.awaitTermination(1, TimeUnit.HOURS);
    -      } catch (InterruptedException e) {
    -        throw new QueryExecutionException(e);
    -      }
    +      queryProperties.executorService.shutdownNow();
    --- End diff --
   
    Please add a comment shutdownnow is called so in case of limit query when number of limit records is already found so executors must stop all the running execution otherwise it will keep running and will hit the query performance.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1679
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/907/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1679
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2416/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1679
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2140/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1679
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2429/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1679
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2161/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1679
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/932/



---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---