You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by paul-rogers <gi...@git.apache.org> on 2016/12/13 01:04:52 UTC

[GitHub] drill pull request #693: DRILL-5122: DrillBuf performs expensive logging if ...

GitHub user paul-rogers opened a pull request:

    https://github.com/apache/drill/pull/693

    DRILL-5122: DrillBuf performs expensive logging if -ea set

    In Drill, once assertions are enabled, the dominant cost of a query
    becomes DrillBuf logging. One example showed that DrillBuf logging
    increases query cost by 10x.
    
    Logging was enabled by the -ea option, which should be available for
    many other uses. Changed to require a specific option to be turned on.
    
    Enabled the option for memory tests (which require the option) but off
    by default and for all other tests.
    
    To do this, converted Surefire system options from command line args to
    system variables so that they can be customized in sub-project poms.

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

    $ git pull https://github.com/paul-rogers/drill DRILL-5122

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

    https://github.com/apache/drill/pull/693.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 #693
    
----
commit 25aa8ea844555806608f1a1e9c30202455219efa
Author: Paul Rogers <pr...@maprtech.com>
Date:   2016-12-12T22:42:07Z

    DRILL-5122: DrillBuf performs expensive logging if -ea set
    
    In Drill, once assertions are enabled, the dominant cost of a query
    becomes DrillBuf logging. One example showed that DrillBuf logging
    increases query cost by 10x.
    
    Logging was enabled by the -ea option, which should be available for
    many other uses. Changed to require a specific option to be turned on.
    
    Enabled the option for memory tests (which require the option) but off
    by default and for all other tests.
    
    To do this, converted Surefire system options from command line args to
    system variables so that they can be customized in sub-project poms.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #693: DRILL-5122: DrillBuf performs expensive logging if ...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/693#discussion_r92472636
  
    --- Diff: exec/memory/base/pom.xml ---
    @@ -40,10 +40,21 @@
         </dependency>
       </dependencies>
     
    -
       <build>
    +    <pluginManagement>
    +      <plugins>
    +        <plugin>
    +          <!-- Configure memory (only) to use the expensive
    +               memory logging. (Tests here fail without the logging. -->
    --- End diff --
    
    Missing closing parenthesis in the comment. (Tests .. )


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #693: DRILL-5122: DrillBuf performs expensive logging if ...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers closed the pull request at:

    https://github.com/apache/drill/pull/693


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #693: DRILL-5122: DrillBuf performs expensive logging if ...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/693#discussion_r92457956
  
    --- Diff: pom.xml ---
    @@ -423,21 +423,23 @@
               <artifactId>maven-surefire-plugin</artifactId>
               <version>2.17</version>
               <configuration>
    -            <argLine>-Xms512m -Xmx3g -Ddrill.exec.http.enabled=false
    -              -Ddrill.exec.sys.store.provider.local.write=false
    -              -Dorg.apache.drill.exec.server.Drillbit.system_options="org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on"
    -              -Ddrill.test.query.printing.silent=true
    -              -Ddrill.catastrophic_to_standard_out=true
    +            <argLine>-Xms512m -Xmx3g
                   -XX:MaxPermSize=512M -XX:MaxDirectMemorySize=3072M
    -              -Djava.net.preferIPv4Stack=true
    -              -Djava.awt.headless=true
                   -XX:+CMSClassUnloadingEnabled -ea</argLine>
                 <forkCount>${forkCount}</forkCount>
                 <reuseForks>true</reuseForks>
                 <additionalClasspathElements>
                   <additionalClasspathElement>./exec/jdbc/src/test/resources/storage-plugins.json</additionalClasspathElement>
                 </additionalClasspathElements>
                 <systemPropertyVariables>
    +              <drill.exec.http.enabled>false</drill.exec.http.enabled>
    +              <drill.exec.sys.store.provider.local.write>false</drill.exec.sys.store.provider.local.write>
    +              <org.apache.drill.exec.server.Drillbit.system_options>"org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on"</org.apache.drill.exec.server.Drillbit.system_options>
    --- End diff --
    
    The line exceeds 120 char limit. Please fix it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #693: DRILL-5122: DrillBuf performs expensive logging if -ea set

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on the issue:

    https://github.com/apache/drill/pull/693
  
    Will fix later.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #693: DRILL-5122: DrillBuf performs expensive logging if ...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/693#discussion_r92462112
  
    --- Diff: pom.xml ---
    @@ -423,21 +423,23 @@
               <artifactId>maven-surefire-plugin</artifactId>
               <version>2.17</version>
               <configuration>
    -            <argLine>-Xms512m -Xmx3g -Ddrill.exec.http.enabled=false
    -              -Ddrill.exec.sys.store.provider.local.write=false
    -              -Dorg.apache.drill.exec.server.Drillbit.system_options="org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on"
    -              -Ddrill.test.query.printing.silent=true
    -              -Ddrill.catastrophic_to_standard_out=true
    +            <argLine>-Xms512m -Xmx3g
                   -XX:MaxPermSize=512M -XX:MaxDirectMemorySize=3072M
    -              -Djava.net.preferIPv4Stack=true
    -              -Djava.awt.headless=true
                   -XX:+CMSClassUnloadingEnabled -ea</argLine>
                 <forkCount>${forkCount}</forkCount>
                 <reuseForks>true</reuseForks>
                 <additionalClasspathElements>
                   <additionalClasspathElement>./exec/jdbc/src/test/resources/storage-plugins.json</additionalClasspathElement>
                 </additionalClasspathElements>
                 <systemPropertyVariables>
    +              <drill.exec.http.enabled>false</drill.exec.http.enabled>
    +              <drill.exec.sys.store.provider.local.write>false</drill.exec.sys.store.provider.local.write>
    +              <org.apache.drill.exec.server.Drillbit.system_options>"org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on"</org.apache.drill.exec.server.Drillbit.system_options>
    +              <drill.test.query.printing.silent>true</drill.test.query.printing.silent>
    +              <drill.catastrophic_to_standard_out>true</drill.catastrophic_to_standard_out>
    +              <drill.memory.debug.allocator>true</drill.memory.debug.allocator>
    +              <java.net.preferIPv4Stack>true</java.net.preferIPv4Stack>
    --- End diff --
    
    "Java.net.preferIPv4Stack" property needs to be set as command line option since it's checked by VM only once at startup. [Reference](https://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html):
    
    And Maven [documentation](http://maven.apache.org/surefire/maven-surefire-plugin/examples/system-properties.html) states that SystemProperties are used only for the ones that can be set after VM startup.
    
    Guess same thing is with "java.awt.headless"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---