You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by sachouche <gi...@git.apache.org> on 2018/03/15 17:09:02 UTC
[GitHub] drill pull request #1168: DRILL-6246: Reduced the size of the jdbc-all jar f...
GitHub user sachouche opened a pull request:
https://github.com/apache/drill/pull/1168
DRILL-6246: Reduced the size of the jdbc-all jar file
- The jdbc-all client jar has been growing in size from version to the next (~20MB in version 1.10, ~27MB in version 1.12, and 34MB in version 1.13
- Note the exact size of the size depends on the maven profile used during compilation (e.g., "mapr" which have more excludes)
- Originally, I tried to exclude the following calcite/avatica packages (responsible for the recent size increase): metrics, proto, org/apache/, com/fasterxml, com/google but some tests failed as the calcite code was referencing some of them
- In this pull request, I have excluded the following packages: calcite/avatica/org and calcite/avatica/metrics
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sachouche/drill DRILL-6246
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/1168.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 #1168
----
commit 6b6188a6c1677595c479f67fc2e33af091c36818
Author: Salim Achouche <sa...@...>
Date: 2018-03-14T02:16:06Z
DRILL-6246: Reduced the size of the jdbc-all jar file
----
---
[GitHub] drill pull request #1168: DRILL-6246: Reduced the size of the jdbc-all jar f...
Posted by sachouche <gi...@git.apache.org>.
Github user sachouche commented on a diff in the pull request:
https://github.com/apache/drill/pull/1168#discussion_r174889287
--- Diff: exec/jdbc-all/pom.xml ---
@@ -473,6 +473,8 @@
<exclude>org/yaml/**</exclude>
<exclude>hello/**</exclude>
<exclude>webapps/**</exclude>
+ <exclude>**/org/apache/calcite/avatica/metrics/**</exclude>
+ <exclude>**/org/apache/calcite/avatica/org/**</exclude>
--- End diff --
done!
---
[GitHub] drill pull request #1168: DRILL-6246: Reduced the size of the jdbc-all jar f...
Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:
https://github.com/apache/drill/pull/1168#discussion_r174882554
--- Diff: exec/jdbc-all/pom.xml ---
@@ -473,6 +473,8 @@
<exclude>org/yaml/**</exclude>
<exclude>hello/**</exclude>
<exclude>webapps/**</exclude>
+ <exclude>**/org/apache/calcite/avatica/metrics/**</exclude>
+ <exclude>**/org/apache/calcite/avatica/org/**</exclude>
--- End diff --
Would be great to make that change. Other than that, changes looks good to me
---
[GitHub] drill issue #1168: DRILL-6246: Reduced the size of the jdbc-all jar file
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/1168
Classes from `avatica.metrics` are used in `JsonHandler`, `ProtobufHandler` and `LocalService`. If Drill does not use these classes than I agree that we can exclude it from `jdbc-all` jar.
Regarding excluding `avatica/org/**`, looks like the problem is in the Avatica pom files since there are no dependencies to `org.apache.commons` and `org.apache.http`, but they are shaded to the jar. Created Jira CALCITE-2215 to fix this issue, but for now, I think it's ok to exclude them.
---
[GitHub] drill issue #1168: DRILL-6246: Reduced the size of the jdbc-all jar file
Posted by sachouche <gi...@git.apache.org>.
Github user sachouche commented on the issue:
https://github.com/apache/drill/pull/1168
I successfully ran the test-suite and some advanced tests which loaded the jdbc artifacts. Is this enough to validate the fix (and future exclusions to keep the jar size in-check)? if not, what other tests(s) do we need to run?
Thanks!
---
[GitHub] drill pull request #1168: DRILL-6246: Reduced the size of the jdbc-all jar f...
Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:
https://github.com/apache/drill/pull/1168#discussion_r174866472
--- Diff: exec/jdbc-all/pom.xml ---
@@ -473,6 +473,8 @@
<exclude>org/yaml/**</exclude>
<exclude>hello/**</exclude>
<exclude>webapps/**</exclude>
+ <exclude>**/org/apache/calcite/avatica/metrics/**</exclude>
+ <exclude>**/org/apache/calcite/avatica/org/**</exclude>
--- End diff --
After removing these exclusion for apache profile what is the size of jdbc-all jar ? based on final size we should reduce the allowed maxSize from ~35MB to that value.
---
[GitHub] drill issue #1168: DRILL-6246: Reduced the size of the jdbc-all jar file
Posted by sachouche <gi...@git.apache.org>.
Github user sachouche commented on the issue:
https://github.com/apache/drill/pull/1168
No I didn't.
Regards,
Salim
Regards,
Salim
-----Original Message-----
From: Kunal Khatua [notifications@github.com]
Received: Tuesday, 10 Apr 2018, 14:01
To: apache/drill [drill@noreply.github.com]
CC: Salim Achouche [sachouche@mapr.com]; Mention [mention@noreply.github.com]
Subject: Re: [apache/drill] DRILL-6246: Reduced the size of the jdbc-all jar file (#1168)
@sachouche<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_sachouche&d=DwMCaQ&c=cskdkSMqhcnjZxdQVpwTXg&r=Yz1mbrEFs0HJzxHJZ69JBLCsKtk9LmaS_d_ncP1HZwY&m=ZuRTiFKVp4XyNeUONWTmOtorxN8OgUnpJeF5Nnbzpi8&s=ORom6yiAaNx9O8pqbz4PEqbCngKTh3c4MO1H6btm4s4&e=> have you had a chance to test the generated JDBC driver with Spotfire/SQuirreL ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_drill_pull_1168-23issuecomment-2D380245152&d=DwMCaQ&c=cskdkSMqhcnjZxdQVpwTXg&r=Yz1mbrEFs0HJzxHJZ69JBLCsKtk9LmaS_d_ncP1HZwY&m=ZuRTiFKVp4XyNeUONWTmOtorxN8OgUnpJeF5Nnbzpi8&s=Q3TGflgHce0_-5S2QAAdrJp8SQnAobk1W0ZFJwD50-Q&e=>, or mute the thread<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AckoK6qKn0CR0-2DszS5136weV3xs2MjZYks5tnR2UgaJpZM4Ssiu1&d=DwMCaQ&c=cskdkSMqhcnjZxdQVpwTXg&r=Yz1mbrEFs0HJzxHJZ69JBLCsKtk9LmaS_d_ncP1HZwY&m=ZuRTiFKVp4XyNeUONWTmOtorxN8OgUnpJeF5Nnbzpi8&s=xgHCT0e9lj785056Kkt2KgkhNR3pvO4wPNI_B3J5S54&e=>.
---
[GitHub] drill issue #1168: DRILL-6246: Reduced the size of the jdbc-all jar file
Posted by sachouche <gi...@git.apache.org>.
Github user sachouche commented on the issue:
https://github.com/apache/drill/pull/1168
@sohami can you please review this pull request?
Thanks!
---
[GitHub] drill pull request #1168: DRILL-6246: Reduced the size of the jdbc-all jar f...
Posted by sachouche <gi...@git.apache.org>.
Github user sachouche commented on a diff in the pull request:
https://github.com/apache/drill/pull/1168#discussion_r174870229
--- Diff: exec/jdbc-all/pom.xml ---
@@ -473,6 +473,8 @@
<exclude>org/yaml/**</exclude>
<exclude>hello/**</exclude>
<exclude>webapps/**</exclude>
+ <exclude>**/org/apache/calcite/avatica/metrics/**</exclude>
+ <exclude>**/org/apache/calcite/avatica/org/**</exclude>
--- End diff --
The Apache jar size has shrunk by ~1MB from over 34MB to 33393541 bytes; I guess we can decrease the limit from 35MB to 34MB.
---
[GitHub] drill issue #1168: DRILL-6246: Reduced the size of the jdbc-all jar file
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on the issue:
https://github.com/apache/drill/pull/1168
I would recommend trying to setup a connection using Spotfire or Squirrel and running a couple of metadata queries and a couple of queries on complex data. (These have traditionally been areas that were affected when the jar was incomplete).
Also try on a secure cluster. You might need help from other folks on some of this.
---
[GitHub] drill issue #1168: DRILL-6246: Reduced the size of the jdbc-all jar file
Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on the issue:
https://github.com/apache/drill/pull/1168
+1 LGTM. Thanks for making the changes.
---
[GitHub] drill issue #1168: DRILL-6246: Reduced the size of the jdbc-all jar file
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:
https://github.com/apache/drill/pull/1168
@sachouche have you had a chance to test the generated JDBC driver with Spotfire/SQuirreL ?
---