[
https://issues.apache.org/jira/browse/DRILL-7790?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17298359#comment-17298359
]
ASF GitHub Bot commented on DRILL-7790:
---------------------------------------
vvysotskyi commented on a change in pull request #2185:
URL: https://github.com/apache/drill/pull/2185#discussion_r590707689
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/UserClientConnection.java
##########
@@ -60,10 +60,10 @@
void sendData(RpcOutcomeListener<Ack> listener, QueryDataPackage data);
/**
- * Returns the {@link ChannelFuture} which will be notified when this
+ * Returns the {@link Future} which will be notified when this
* channel is closed. This method always returns the same future instance.
*/
- ChannelFuture getChannelClosureFuture();
+ Future getClosureFuture();
Review comment:
`Future` is parametrized, please specify the `Void` type to avoid
warnings. `ChannelFuture` extends `Future<Void>`.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java
##########
@@ -284,14 +282,9 @@ public void sendData(final RpcOutcomeListener<Ack>
listener, final QueryDataPack
}
@Override
- public ChannelFuture getChannelClosureFuture() {
+ public Future getClosureFuture() {
Review comment:
And here.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/work/prepare/PreparedStatementProvider.java
##########
@@ -244,8 +244,8 @@ public UserSession getSession() {
}
@Override
- public ChannelFuture getChannelClosureFuture() {
- return inner.getChannelClosureFuture();
+ public Future getClosureFuture() {
Review comment:
And here (Void).
##########
File path:
exec/memory/base/src/main/java/org/apache/drill/exec/memory/DrillByteBufAllocator.java
##########
@@ -111,31 +118,68 @@ public boolean isDirectBufferPooled() {
@Override
public ByteBuf heapBuffer() {
- throw fail();
+ return HEAP_ALLOCATOR.allocateHeap();
}
@Override
public ByteBuf heapBuffer(int initialCapacity) {
- throw fail();
+ return HEAP_ALLOCATOR.allocateHeap(initialCapacity);
}
@Override
public ByteBuf heapBuffer(int initialCapacity, int maxCapacity) {
- throw fail();
+ return HEAP_ALLOCATOR.allocateHeap(initialCapacity, maxCapacity);
}
@Override
public CompositeByteBuf compositeHeapBuffer() {
- throw fail();
+ return compositeHeapBuffer(DEFAULT_MAX_COMPOSITE_COMPONENTS);
}
@Override
public CompositeByteBuf compositeHeapBuffer(int maxNumComponents) {
- throw fail();
- }
-
- private RuntimeException fail() {
- throw new UnsupportedOperationException("Allocator doesn't support
heap-based memory.");
+ return new CompositeByteBuf(this, false, maxNumComponents);
+ }
+
+ /**
+ * This method was copied from AbstractByteBufAllocator. Netty 4.1.x moved
this method from
+ * AbstractByteBuf to AbstractByteBufAllocator. However, as
DrillByteBufAllocator doesn't extend
+ * AbstractByteBufAllocator, it doesn't get the implementation automatically
and we have to copy
+ * the codes.
+ */
+ @Override
+ public int calculateNewCapacity(int minNewCapacity, int maxCapacity) {
Review comment:
Please remove this method copy and implementations above, and extend
this class from `AbstractByteBufAllocator` (`newHeapBuffer` and
`newDirectBuffer` implementations may be implemented as
`HEAP_ALLOCATOR.allocateXXX`).
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebSessionResources.java
##########
@@ -58,7 +58,7 @@ public BufferAllocator getAllocator() {
return allocator;
}
- public ChannelPromise getCloseFuture() {
+ public Promise getCloseFuture() {
Review comment:
And here (Void).
##########
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/WebSessionResourcesTest.java
##########
@@ -65,15 +64,13 @@ public void operationComplete(Future<Void> future) throws
Exception {
@Test
public void testChannelPromiseWithNullExecutor() throws Exception {
try {
- ChannelPromise closeFuture = new DefaultChannelPromise(null);
+ Promise closeFuture = new DefaultPromise(null);
Review comment:
And here (Void).
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
##########
@@ -283,12 +283,12 @@ public WebUserConnection provide() {
logger.trace("Failed to get the remote address of the http session
request", ex);
}
- // Create a dummy close future which is needed by Foreman only. Foreman
uses this future to add a close
+ // Create a close future which is needed by Foreman only. Foreman uses
this future to add a close
// listener to known about channel close event from underlying layer.
//
// The invocation of this close future is no-op as it will be triggered
after query completion in unsecure case.
// But we need this close future as it's expected by Foreman.
- final ChannelPromise closeFuture = new DefaultChannelPromise(null,
executor);
+ final Promise closeFuture = new DefaultPromise(executor);
Review comment:
And here (Void).
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/BaseWebUserConnection.java
##########
@@ -40,7 +39,7 @@ public UserSession getSession() {
}
@Override
- public ChannelFuture getChannelClosureFuture() {
+ public Future getClosureFuture() {
Review comment:
And here.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
##########
@@ -116,7 +115,7 @@
private final ResponseSendListener responseListener = new
ResponseSendListener();
private final GenericFutureListener<Future<Void>> closeListener = future ->
cancel();
- private final ChannelFuture closeFuture;
+ private final Future closeFuture;
Review comment:
And here (Void).
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
##########
@@ -221,11 +221,11 @@ public WebUserConnection provide() {
config.getLong(ExecConstants.HTTP_SESSION_MEMORY_RESERVATION),
config.getLong(ExecConstants.HTTP_SESSION_MEMORY_MAXIMUM));
- // Create a dummy close future which is needed by Foreman only.
Foreman uses this future to add a close
+ // Create a future which is needed by Foreman only. Foreman uses this
future to add a close
// listener to known about channel close event from underlying layer.
We use this future to notify Foreman
// listeners when the Web session (not connection) between Web Client
and WebServer is closed. This will help
// Foreman to cancel all the running queries for this Web Client.
- final ChannelPromise closeFuture = new DefaultChannelPromise(null,
executor);
+ final Promise closeFuture = new DefaultPromise(executor);
Review comment:
And here (Void).
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebSessionResources.java
##########
@@ -40,10 +40,10 @@
private final UserSession webUserSession;
- private ChannelPromise closeFuture;
+ private Promise closeFuture;
Review comment:
And here (Void).
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebSessionResources.java
##########
@@ -40,10 +40,10 @@
private final UserSession webUserSession;
- private ChannelPromise closeFuture;
+ private Promise closeFuture;
WebSessionResources(BufferAllocator allocator, SocketAddress remoteAddress,
- UserSession userSession, ChannelPromise closeFuture) {
+ UserSession userSession, Promise closeFuture) {
Review comment:
And here (Void).
----------------------------------------------------------------
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:
[email protected]
> Build Drill with Netty version 4.1.50.Final
> -------------------------------------------
>
> Key: DRILL-7790
> URL: https://issues.apache.org/jira/browse/DRILL-7790
> Project: Apache Drill
> Issue Type: Bug
> Affects Versions: 1.17.0
> Reporter: alka kumari
> Assignee: Rymar Maksym
> Priority: Major
>
> Hi,
>
> In apache Drill Client 1.17, Netty version 4.0.48.Final is being used and it
> suffers from vulnerability (CVE-2019-16869):
> https://www.cvedetails.com/cve/CVE-2019-16869/
> https://snyk.io/vuln/maven:io.netty%3Anetty-all
>
> This has been fixed in the latest netty (4.1.50.Final).
>
> We want to build a drill with the latest Netty version that is free from any
> vulnerabilities.
>
> As there are many breaking changes from 4.0.48 to 4.1.50, I have modified the
> code accordingly.
>
> I noticed that after trying to upgrade the dependency, I was unable to
> connect with SSL enabled.
>
> ERROR:
> Connecting to the server timed out. This is sometimes due to a mismatch in
> the SSL configuration between client and server. [ Exception: Waited 10000
> milliseconds for
> org.apache.drill.shaded.guava.com.google.common.util.concurrent.SettableFuture@6ea2bc93[status=PENDING]].
>
>
> I have created a pull request containing the changes which I have tried to
> make.
>
> Could someone please advise further on what needs to be changed?
>
> Regards,
> Alka
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
