[GitHub] carbondata pull request #2889: [CARBONDATA-3067] Add check for debug to avoi...

[qiuchenjian-2]

GitHub user xuchuanyin opened a pull request:

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

    [CARBONDATA-3067] Add check for debug to avoid string concat

    For bloomfilter datamap, it will log debug for each blocklet. If the
    data is huge, there maybe thousands of blocklet, so in this commit, we
    will try to avoid necessary string concat if the debug level is not
    enabled.
   
    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/xuchuanyin/carbondata 101101_opt_debug_log

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

    https://github.com/apache/carbondata/pull/2889.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 #2889
   
----
commit d29cd6e38048851fc65ce1f713f534a51154385b
Author: xuchuanyin <xuchuanyin@...>
Date:   2018-11-01T02:09:40Z

    Add check for debug to avoid string concat
   
    For bloomfilter datamap, it will log debug for each blocklet. If the
    data is huge, there maybe thousands of blocklet, so in this commit, we
    will try to avoid necessary string concat if the debug level is not
    enabled.

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/2889
 
    There is some frequent info log in `UnsafeSortMemoryManager`, can you also add if check for that


---

[qiuchenjian-2]

Github user xuchuanyin commented on the issue:

    https://github.com/apache/carbondata/pull/2889
 
    I think there is no need to add check for info level logs and the debug level logs in those class already have corresponding check.
   
    I  think current logs in `UnsafeSortMemoryManager` and `UnsafeMemoryManager` are OK now. In which part do you think may has problems.


---

[qiuchenjian-2]

Github user xuchuanyin commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/2889
 
    In `UnsafeMemoryManager:175`, there is a info log that requires constructing a string


---

[qiuchenjian-2]

Github user xuchuanyin commented on the issue:

    https://github.com/apache/carbondata/pull/2889
 
    @jackylk Do you want to change that to `debug`? Currently it is info level which is not in the scope of this PR since info log may always be logged and that concatenation is unavoidable.


---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/2889
 
    ok. I think we do not need to change it now


---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/2889
 
    LGTM


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---