[
https://issues.apache.org/jira/browse/DRILL-7393?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16986421#comment-16986421
]
ASF GitHub Bot commented on DRILL-7393:
---------------------------------------
paul-rogers commented on issue #1910: DRILL-7393: Revisit Drill tests to ensure
that patching is executed b…
URL: https://github.com/apache/drill/pull/1910#issuecomment-560733231
@agozhiy, thank you, the explanation is crystal clear. In fact, I think I've
been seeing a similar issue with the ZK and Dynamic UDF tests: they randomly
fail about 75% of the time due to issues associated with loading/finding
classes that appear to be generated on the fly.
The test solution might solve the above race conditions if all tests invoke
the same patcher as their first task. We would have to assume that the Drillbit
does also. But, even then, we *still* would not solve the problem. SqlLine
runs, then loads any number of JDBC drivers (in the general case) followed by
the Drill JDBC driver which can, if requested, start a Drillbit. We have little
control over when patching is done in this case (unless we add patching to
SqlLine startup.)
Generalizing, since any app can load the Drill JDBC driver and run Drill,
*every* app would need the magic patcher run at startup. This seems like a
solution that won't scale.
We're talking about patching at runtime. I wonder, can we patch at build
time? Can we create a Drill-specific version of Guava with the patches applied,
stored in a jar file?
This still does not help with third-party apps: they might put their own
Guava on the class path before the patched Drill version.
Did some Googling for solutions since we can't be the only folks with the
problem. You've probably done the same. Here is a [note from the Guava
team](https://github.com/google/guava/wiki/UseGuavaInYourBuild) which
recommends using Guava version 21.
> In the simple case, just upgrade your project to the newest version of
Guava. ... However, this can fail in some cases:
>
> *One of your dependencies uses a version of Guava from the days when we
were still removing non-@Beta APIs.
>
> In these cases, you may be able to find an older version of Guava that
satisfies all your dependencies. However, the only general solution is for all
projects to update to at least Guava 21...
There seems to be a [Java library compliance checker
tool](https://github.com/lvc/japi-compliance-checker) that might help identify
actual gaps.
It also seems that that the only real solution that folks mention is
[shading](https://bryantsai.com/how-to-resolve-dependency-conflict-out-of-your-control-e75ace79e54f).
Can we shade the MapR plugin along with its dependent obsolete Guava version?
Otherwise, the only reliable solution is to get the set of dependencies to
use compatible Guava versions. Can we upgrade our Hive/Hadoop dependencies to
newer versions that depend on recent Guava versions?
Look at it from the point of view of MapR and Hadoop: they cannot expect
their clients to be on an ancient version of Guava; to support their users
(including Drill), they have to stay current. Perhaps a bit of lobbying,
searching will solve the problem without patching?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Revisit Drill tests to ensure that patching is executed before any test run
> ---------------------------------------------------------------------------
>
> Key: DRILL-7393
> URL: https://issues.apache.org/jira/browse/DRILL-7393
> Project: Apache Drill
> Issue Type: Task
> Affects Versions: 1.16.0, 1.17.0
> Reporter: Arina Ielchiieva
> Assignee: Anton Gozhiy
> Priority: Major
> Labels: ready-to-commit
>
> Apache Drill patches some Protobuf and Guava classes (see GuavaPatcher,
> ProtobufPatcher), patching should be done before classes to be patched are
> loaded. That's why this operation is executed in static block in Drillbit
> class.
> Some tests in java-exec module use Drillbit class, some extend DrillTest
> class, both of them patch Guava. But there are some tests that do not call
> patcher but load classes to be patched. For example,
> {{org.apache.drill.exec.sql.TestSqlBracketlessSyntax}} loads Guava
> Preconditions class. If such tests run before tests that require patching,
> tests run will fail since patching won't be successful. Patchers code does
> not fail application if patching was not complete, just logs warning
> ({{logger.warn("Unable to patch Guava classes.", e);}}), so sometimes it hard
> to identify unit tests failure root cause.
> We need to revisit all Drill tests to ensure that all of them extend common
> test base class which patchers Protobuf and Guava classes in static block.
> Also refactor Patcher classes to have assert to fail if patching fails during
> unit testing if there are any problems.
> After all tests are revised, we can remove {{metastore-test}} execution from
> main.xml in {{maven-surefire-plugin}} which was added to ensure that all
> Metastore tests run in a separate JVM where patching is done in first place
> since Iceberg Metastore heavily depends on patched Guava Preconditions class.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
