You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2022/07/22 14:09:18 UTC
[GitHub] [drill] jnturton opened a new pull request, #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
jnturton opened a new pull request, #2610:
URL: https://github.com/apache/drill/pull/2610
# [DRILL-8268](https://issues.apache.org/jira/browse/DRILL-8268): Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
## Description
New exclusions of slf4j-reload4j are required in the Hadoop 2 profile, probably due to the upgrade of Hadoop from 2.10.1 to 2.10.2. The netty-all metapackage entered the dependency tree with the change introducing the Netty bom bringing many uneeded libs with it.
The heap memory usage limiting logic in the REST server is disabled by default since query results are streamed. This change aims to let the Java GC to now do its job without interference and if that results in OOM under a constant load then there is good evidence for a heap leak which must be tracked and completely resolved anyway, not mitigated or "swept under a rug".
## Documentation
N/A
## Testing
Lib exclusions: successful build and launch under the Hadoop 2 profile
Heap memory usage: new (or existing) performance test in the drill-test-framework.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r929457387
##########
exec/vector/pom.xml:
##########
@@ -25,7 +25,7 @@
<groupId>org.apache.drill.exec</groupId>
<version>2.0.0-SNAPSHOT</version>
</parent>
- <artifactId>vector</artifactId>
+ <artifactId>drill-vector</artifactId>
Review Comment:
@luocooong it is only the artifactId that is inconsistent here. The name of `Drill : Exec : Vectors` already looks good to me. So I'll revert this. I agree that the renaming could not be backported like the rest of the changes here are meant to be.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r930036634
##########
pom.xml:
##########
@@ -3965,6 +3977,46 @@
</exclusion>
</exclusions>
</dependency>
+ <dependency>
Review Comment:
My audit of jars that were not present in 1.20.1-hadoop2
#### Comes from Google Sheets plugin
Only in /opt/apache-drill/jars/3rdparty: opencensus-api-0.31.1.jar
Only in /opt/apache-drill/jars/3rdparty: opencensus-contrib-http-util-0.31.1.jar
Only in /opt/apache-drill/jars/3rdparty: grpc-context-1.27.2.jar
Only in /opt/apache-drill/jars/3rdparty: google-api-client-1.35.2.jar
Only in /opt/apache-drill/jars/3rdparty: google-api-client-jackson2-1.35.2.jar
Only in /opt/apache-drill/jars/3rdparty: google-api-services-sheets-v4-rev20220606-1.32.1.jar
Only in /opt/apache-drill/jars/3rdparty: google-auth-library-credentials-1.8.0.jar
Only in /opt/apache-drill/jars/3rdparty: google-auth-library-oauth2-http-1.8.0.jar
Only in /opt/apache-drill/jars/3rdparty: google-http-client-1.42.0.jar
Only in /opt/apache-drill/jars/3rdparty: google-http-client-apache-v2-1.42.0.jar
Only in /opt/apache-drill/jars/3rdparty: google-http-client-gson-1.42.0.jar
Only in /opt/apache-drill/jars/3rdparty: google-oauth-client-1.34.1.jar
Only in /opt/apache-drill/jars/3rdparty: google-oauth-client-java6-1.34.1.jar
Only in /opt/apache-drill/jars/3rdparty: google-oauth-client-jetty-1.34.1.jar
#### Comes from Swagger
Only in /opt/apache-drill/jars/3rdparty: narcissus-1.0.1.jar
Only in /opt/apache-drill/jars/3rdparty: jvm-driver-4.0.0.jar
Only in /opt/apache-drill/jars/3rdparty: classgraph-4.8.117.jar
Only in /opt/apache-drill/jars/3rdparty: swagger-annotations-2.1.12.jar
Only in /opt/apache-drill/jars/3rdparty: swagger-core-2.1.12.jar
Only in /opt/apache-drill/jars/3rdparty: swagger-integration-2.1.12.jar
Only in /opt/apache-drill/jars/3rdparty: swagger-jaxrs2-2.1.12.jar
Only in /opt/apache-drill/jars/3rdparty: swagger-jaxrs2-servlet-initializer-v2-2.1.12.jar
Only in /opt/apache-drill/jars/3rdparty: swagger-models-2.1.12.jar
#### Looks like nimbus-jose-jwt was downgraded in Hadoop 2, causing this downgrade
Only in /opt/apache-drill/jars/3rdparty: accessors-smart-1.2.jar
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r933069130
##########
exec/jdbc-all/pom.xml:
##########
@@ -206,6 +231,38 @@
<groupId>io.swagger.core.v3</groupId>
<artifactId>swagger-jaxrs2-servlet-initializer-v2</artifactId>
</exclusion>
+ <exclusion>
+ <groupId>com.squareup.okhttp3</groupId>
+ <artifactId>okhttp</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>com.tdunning</groupId>
+ <artifactId>t-digest</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>com.bettercloud</groupId>
+ <artifactId>vault-java-driver</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>com.esri.geometry</groupId>
+ <artifactId>esri-geometry-api</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>com.yahoo.datasketches</groupId>
+ <artifactId>sketches-core</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.htrace</groupId>
+ <artifactId>htrace-core4</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>${calcite.groupId}</groupId>
+ <artifactId>calcite-core</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>${calcite.groupId}</groupId>
+ <artifactId>calcite-linq4j</artifactId>
Review Comment:
No need to exclude Calcite, and perhaps some other dependencies here, since they are already excluded from the target JAR in the configuration for `maven-shade-plugin`.
##########
exec/jdbc-all/pom.xml:
##########
@@ -329,8 +386,6 @@
</includes>
<excludes>
<exclude>io.protostuff:*</exclude>
- <exclude>${calcite.groupId}:calcite-core</exclude>
Review Comment:
It is better to have exclusions here since they are applied to all dependencies.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r932494153
##########
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/sequencefile/SequenceFileBatchReader.java:
##########
@@ -154,6 +153,19 @@ public boolean next() {
@Override
public void close() {
- AutoCloseables.closeSilently(reader);
+ try {
+ // Hadoop 2 compat: {@link org.apache.hadoop.mapred.RecordReader} does not
+ // support AutoCloseable and must be closed manually.
+ if (reader != null) {
+ reader.close();
+ reader = null;
+ }
+ } catch (IOException e) {
+ throw UserException
Review Comment:
`AutoCloseables.closeSilently` doesn't throw any errors.
##########
exec/jdbc-all/pom.xml:
##########
@@ -1230,7 +1230,7 @@
<profile>
<id>hadoop-2</id>
<properties>
- <jdbc-all-jar.maxsize>50400000</jdbc-all-jar.maxsize>
+ <jdbc-all-jar.maxsize>52800000</jdbc-all-jar.maxsize>
Review Comment:
Can we exclude some unnecessary dependencies from JDBC Driver to avoid its inflation?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] martin-g commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
martin-g commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1193986421
@luocooong See https://stackoverflow.com/a/61267496/497381
` java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer;` is not available in JDK 1.8. It is available since Java 11.
In 1.8 the return type is `Ljava/nio/Buffer;`.
I am not sure why Github CI / Main Build (8)` passes. Maybe it does not cover this code path ?!
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1196540449
> @jnturton, looks like this PR causes extra warnings:
Thanks for catching this, I believe it's fixed now.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r928870530
##########
exec/vector/pom.xml:
##########
@@ -25,7 +25,7 @@
<groupId>org.apache.drill.exec</groupId>
<version>2.0.0-SNAPSHOT</version>
</parent>
- <artifactId>vector</artifactId>
+ <artifactId>drill-vector</artifactId>
Review Comment:
@luocooong it was seeing the effort already made in [DRILL-7847](https://issues.apache.org/jira/browse/DRILL-7847) that caused me to try to complete the job. I do like completeness and consistency but the name of the vector module also does not keep me awake at night so I won't argue much here. Do you recommend that I revert this part of the PR?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1196632952
@luocooong @vvysotskyi, so the Kerberos unit tests that we recently got working and enabled for the default-hadoop profile (#2592) remain flaky in the new hadoop-2 CI profile. I've just added a commit to exclude them from that profile.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r932977164
##########
exec/jdbc-all/pom.xml:
##########
@@ -1230,7 +1230,7 @@
<profile>
<id>hadoop-2</id>
<properties>
- <jdbc-all-jar.maxsize>50400000</jdbc-all-jar.maxsize>
+ <jdbc-all-jar.maxsize>52800000</jdbc-all-jar.maxsize>
Review Comment:
Always :-)
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1197713664
@luocooong @vvysotskyi there was a lot of noise in the pom.xml diff. I've now redone the changes to it to remove that.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1193860317
@luocooong @paul-rogers a bit of data related to the heap memory usage limiting from some experiments run by a helpful user.
I did test the recommendation given by you and it worked well. Below are steps of testing
> drill.exec.http.memory.heap.failure.threshold = 2
>
> Before making changes in the drill-override.conf
> 1. Opened 4 sessions of drill web UI query
> 2. Fired same query in all 4 sessions. This query will scan through 8000 parquet compress files
>
> Results - all 4 sessions gave heap error.
> Observations
> 1. The heap memory displayed on "metrics" tab varies between 30-80% while query is running
> 2. The heap memory is constant at 87.89% after queries are failed
>
> After making the changes in the drill-overide.conf as recommended by you
> 1. Opened 4 sessions of drill web UI query
> 2. Fired same query in all 4 sessions. This query will scan through 8000 parquet compress files
> Results - All 4 sessions returned the results. It took average 6+ minutes to return the results
> Observations
> 1. Could not observe the heap memory displayed on "metrics" as display was just in running state and then gave error "page cannot be displayed" during query run
> 2. The heap memory is varying between 69.71% after queries are successfully completed
>
> I ran the same queries again and again and below is the heap memory after every run
> 2nd run - Between 61-62%
> 3rd run - . Between 82-83%
> 4th Run - Between 69-70%
> 5th Run - Between 65-66%
> 6th run - Between 69-70%
> 7th run - Between 70-71%
> 8th run - increased one session, so now total are 5 web sessions - 1 session gave "Page cannot be displayed error. 4 returned the results - Memory 79-80%
> 9th run (5 sessions) - - 1 session gave "Page cannot be displayed error. 4 returned the results - Memory 66-67%
>
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
luocooong commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r928797183
##########
exec/vector/pom.xml:
##########
@@ -25,7 +25,7 @@
<groupId>org.apache.drill.exec</groupId>
<version>2.0.0-SNAPSHOT</version>
</parent>
- <artifactId>vector</artifactId>
+ <artifactId>drill-vector</artifactId>
Review Comment:
Change is not the best thing without more new things.
##########
pom.xml:
##########
@@ -3965,6 +3977,46 @@
</exclusion>
</exclusions>
</dependency>
+ <dependency>
Review Comment:
The dependence on Hadoop was tricky. Ensure that the dependency tree does not have a new dependency.
##########
exec/java-exec/src/main/resources/drill-module.conf:
##########
@@ -190,7 +190,9 @@ drill.exec: {
maximum: 9223372036854775807
}
},
- memory.heap.failure.threshold: 0.85,
+ # Default to failing queries only at 100% heap usage, i.e. the heap usage
+ # limiting logic in the REST API is disabled by default.
+ memory.heap.failure.threshold: 1.0,
Review Comment:
That's okay.
In fact, this option is only used for REST modules and it has little effect before Streaming RESTful feature joins because we cannot expect users to send long or short queries.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1195743563
@jnturton, looks like this PR causes extra warnings:
```
2022-07-26T14:35:28.5702064Z [WARNING]
2022-07-26T14:35:28.5703626Z [WARNING] Some problems were encountered while building the effective model for org.apache.drill.tools:tools-parent:pom:2.0.0-SNAPSHOT
2022-07-26T14:35:28.5706084Z [WARNING] 'profiles.profile[default-hadoop].dependencyManagement.dependencies.dependency.(groupId:artifactId:type:classifier)' must be unique: org.apache.hadoop:hadoop-common:jar -> duplicate declaration of version ${hadoop.version} @ org.apache.drill:drill-root:2.0.0-SNAPSHOT, /home/runner/work/drill/drill/pom.xml, line 2246, column 23
2022-07-26T14:35:28.5707420Z [WARNING]
2022-07-26T14:35:28.5708538Z [WARNING] Some problems were encountered while building the effective model for org.apache.drill:drill-root:pom:2.0.0-SNAPSHOT
2022-07-26T14:35:28.5709650Z [WARNING] 'profiles.profile[default-hadoop].dependencyManagement.dependencies.dependency.(groupId:artifactId:type:classifier)' must be unique: org.apache.hadoop:hadoop-common:jar -> duplicate declaration of version ${hadoop.version} @ line 2246, column 23
2022-07-26T14:35:28.5710552Z [WARNING]
2022-07-26T14:35:28.5711039Z [WARNING] It is highly recommended to fix these problems because they threaten the stability of your build.
2022-07-26T14:35:28.5711458Z [WARNING]
2022-07-26T14:35:28.5711917Z [WARNING] For this reason, future Maven versions might no longer support building such malformed projects.
```
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r932878062
##########
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/sequencefile/SequenceFileBatchReader.java:
##########
@@ -154,6 +153,19 @@ public boolean next() {
@Override
public void close() {
- AutoCloseables.closeSilently(reader);
+ try {
+ // Hadoop 2 compat: {@link org.apache.hadoop.mapred.RecordReader} does not
+ // support AutoCloseable and must be closed manually.
+ if (reader != null) {
+ reader.close();
+ reader = null;
+ }
+ } catch (IOException e) {
+ throw UserException
Review Comment:
@vvysotskyi the implementation of close() in this PR restores what was there up until a few months ago when AutoCloseables came in (#2498). So we have only been swallowing errors here since March, we never used to (and the stable branch of Drill still does not swallow errors here since #2498 was not backported). Should I nevertheless change the exception handling logic to swallow errors?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r933186066
##########
exec/jdbc-all/pom.xml:
##########
@@ -329,8 +386,6 @@
</includes>
<excludes>
<exclude>io.protostuff:*</exclude>
- <exclude>${calcite.groupId}:calcite-core</exclude>
Review Comment:
When they're made conventional Maven exclusions then they act transitively resulting in size savings. They also make the mvn depedency:tree tool more useful.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1193350318
@jnturton, looks like this change causes Travis CI failures:
```
(java.lang.RuntimeException) java.lang.NoSuchMethodError: java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer;
52991 org.apache.drill.common.DeferredException.addThrowable():101
52992 org.apache.drill.exec.work.fragment.FragmentExecutor.fail():502
52993 org.apache.drill.exec.work.fragment.FragmentExecutor.run():347
52994 org.apache.drill.common.SelfCleaningRunnable.run():38
52995 java.util.concurrent.ThreadPoolExecutor.runWorker():1149
52996 java.util.concurrent.ThreadPoolExecutor$Worker.run():624
52997 java.lang.Thread.run():748
52998 Caused By (java.lang.NoSuchMethodError) java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer;
52999 org.apache.drill.exec.vector.UInt4Vector$BufferedMutator.setSafe():635
53000 org.apache.drill.exec.vector.VarBinaryVector$BufferedMutator.<init>():922
53001 org.apache.drill.exec.vector.VarBinaryVector$BufferedMutator.<init>():898
53002 org.apache.drill.exec.vector.VarBinaryVector$Mutator.setSafe():642
53003 org.apache.drill.exec.vector.VarBinaryVector$Mutator.setSafe():630
53004 org.apache.drill.exec.store.parquet.columnreaders.VarLengthColumnReaders$VarBinaryColumn.setSafe():268
53005 org.apache.drill.exec.store.parquet.columnreaders.VarLengthValuesColumn.readRecordsInBulk():98
53006 org.apache.drill.exec.store.parquet.columnreaders.VarLenBinaryReader.readRecordsInBulk():113
53007 org.apache.drill.exec.store.parquet.columnreaders.VarLenBinaryReader.readFields():91
53008 org.apache.drill.exec.store.parquet.columnreaders.BatchReader$VariableWidthReader.readRecords():156
```
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
luocooong commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r929442941
##########
exec/vector/pom.xml:
##########
@@ -25,7 +25,7 @@
<groupId>org.apache.drill.exec</groupId>
<version>2.0.0-SNAPSHOT</version>
</parent>
- <artifactId>vector</artifactId>
+ <artifactId>drill-vector</artifactId>
Review Comment:
Good point.
It is recommended to update the name of the module instead of the ID, and consistency will do better. Because we also need to treat this pull request that allows backport.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a diff in pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on code in PR #2610:
URL: https://github.com/apache/drill/pull/2610#discussion_r929457387
##########
exec/vector/pom.xml:
##########
@@ -25,7 +25,7 @@
<groupId>org.apache.drill.exec</groupId>
<version>2.0.0-SNAPSHOT</version>
</parent>
- <artifactId>vector</artifactId>
+ <artifactId>drill-vector</artifactId>
Review Comment:
@luocooong it is only the artifactId that is inconsistent here. The name of `Drill : Exec : Vectors` already looks good to me. So I'll revert this.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1193118355
@luocooong the main goal of this PR is to solve issues in RC0 of Drill 1.20.2 and produce RC1.
Unrelated to that, I can expand a bit more on my reasoning for the commit disabling the heap memory usage limiter in the REST server by default. Since we stream rather than buffer REST API query results now, we no longer have an excuse for runaway heap usage. Further, while preventing the next REST query from running can save the Drillbit from an OOM death, when the user complains that they cannot run their query an admin is very likely to restart that Drillbit _anyway_. So we probably only delayed the inevitable even though the landing was made a bit softer and other processing on the Drillbit may have been able to continue.
Since now the only things that can cause runaway heap usage (that I can see) under a constant load are leaks, unecessary object references in Java's case, I think we should bite the bullet and fail hard with OOM when we exhaust the heap. The Drillbit can easily be reincarnated upon its death by systemd or another supervisor, which is preferable to having it limp on declining queries arriving over REST. We also get to stop applying our own heap management rule which may interact badly with the GC in use. E.g. if we start turning REST queries away at 85% heap usage but 90% heap usage would have stirred the user's GC to do a deep clean (in reality I think they all kick in considerably earlier but I'm really making a design argument here) then we are working against our own JVM. Lastly any ensuing bug reports overtly containing "OutOfMemoryError" are more likely, I believe, to send us hunting heap memory leaks.
cc @paul-rogers.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] martin-g commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
martin-g commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1195684710
Good job, James!
On Tue, Jul 26, 2022, 18:37 James Turton ***@***.***> wrote:
> @luocooong <https://github.com/luocooong> See
> https://stackoverflow.com/a/61267496/497381
> java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer; is not available in
> JDK 1.8. It is available since Java 11. In 1.8 the return type is
> Ljava/nio/Buffer;. I am not sure why Github CI / Main Build (8) passes.
> Maybe it does not cover this code path ?!
>
> @martin-g <https://github.com/martin-g> thanks for looking into it, we've
> progressed to a successful build now. I was dragging in bad dependencies
> and somehow, specifically for aarch64, that seems to have caused a jar
> compiled with a >8 JDK, probably Netty, to have been pulled in to the
> Travis JDK 8 environment. That's my suspicion, anyway.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/drill/pull/2610#issuecomment-1195642829>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AABYUQQ54KHMPPSGE74NRXTVWAA2XANCNFSM54LVHSKQ>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton merged pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton merged PR #2610:
URL: https://github.com/apache/drill/pull/2610
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1199209490
@vvysotskyi JDBC clients tested after new exclusions:
sqlline 1.12
SquirrelSQL 4.40
DBeaver 22.0.2 installed as an Eclipse plugin
All tests had htpasswd authentication and SSL enabled.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2610: DRILL-8268: Fix Hadoop 2 and Netty lib exclusions, REST mem limiter disabled by default
Posted by GitBox <gi...@apache.org>.
jnturton commented on PR #2610:
URL: https://github.com/apache/drill/pull/2610#issuecomment-1195642829
> @luocooong See https://stackoverflow.com/a/61267496/497381 ` java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer;` is not available in JDK 1.8. It is available since Java 11. In 1.8 the return type is `Ljava/nio/Buffer;`. I am not sure why `Github CI / Main Build (8)` passes. Maybe it does not cover this code path ?!
@martin-g thanks for looking into it, we've progressed to a successful build now. I was dragging in bad dependencies and somehow, specifically for aarch64, that seems to have caused a jar compiled with a >8 JDK, probably Netty, to have been pulled in to the Travis JDK 8 environment. That's my suspicion, anyway.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org