You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by "rymarm (via GitHub)" <gi...@apache.org> on 2023/12/13 20:49:57 UTC

[PR] Update Netty to 4.1.101 (drill)

rymarm opened a new pull request, #2857:
URL: https://github.com/apache/drill/pull/2857

   # [DRILL-8467](https://issues.apache.org/jira/browse/DRILL-8467): Update netty to 4.1.101
   
   ## Description
   Update Netty to `4.1.101`. 
   Netty `4.1.75` has 2 "breaking" changes:
   1. The default `PooledByteBufAllocator` chunk size was reduced from `16 MiB` to `4 MiB`
   2. The default value of `io.netty.allocator.useCacheForAllThreads` was changed to `false`  
      
   Source: https://netty.io/news/2022/03/10/4-1-75-Final.html
   
   I made `InnerAllocator` creation with the previously accepted chunk size for Drill - 16 MiB, and left `useCacheForAllThreads` enabled because as far as I understand cache for all threads(not only for the Netty theads) gives better performance for Drill case. 
   
   I left those options overridable by Netty Java properties `io.netty.allocator.useCacheForAllThreads` and `io.netty.allocator.maxOrder`([responds for chunk size](https://github.com/netty/netty/blob/c29508672c7e9aa01bbe2a14f7438d82c5b25f91/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java#L104)), so it can be changed without Drill recompilation as it was before.
   
   ## Documentation
   No changes
   
   ## Testing
   Run unit tests.
   


-- 
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


Re: [PR] DRILL-8467: Update Netty to 4.1.101 (drill)

Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton merged PR #2857:
URL: https://github.com/apache/drill/pull/2857


-- 
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


Re: [PR] DRILL-8467: Update Netty to 4.1.101 (drill)

Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on PR #2857:
URL: https://github.com/apache/drill/pull/2857#issuecomment-1872959767

   > P.S. You decide to finish all unresolved tasks until the end of the year?)
   
   😂


-- 
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


Re: [PR] DRILL-8467: Update Netty to 4.1.101 (drill)

Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on PR #2857:
URL: https://github.com/apache/drill/pull/2857#issuecomment-1872986748

   We just need to bump up the JAR size limit in the hadoop-2 profile at the bottom of the pom in jdbc-all 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


Re: [PR] Update Netty to 4.1.101 (drill)

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #2857:
URL: https://github.com/apache/drill/pull/2857#issuecomment-1859219199

   @rymarm Would it make sense to add config options for these "new" parameters?


-- 
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


Re: [PR] DRILL-8467: Update Netty to 4.1.101 (drill)

Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on PR #2857:
URL: https://github.com/apache/drill/pull/2857#issuecomment-1872869093

   @rymarm I just got [the same test failure in the 1.21 branch](https://github.com/apache/drill/actions/runs/7368854517/job/20053552574?pr=2860) which is still based on Netty 4.1.73.Final. I'm testing there with more direct memory and will send a PR to master if that takes care of the issue.


-- 
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


Re: [PR] Update Netty to 4.1.101 (drill)

Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on PR #2857:
URL: https://github.com/apache/drill/pull/2857#issuecomment-1872673208

   Hi @rymarm! Here are two things it would be interesting to redo the Java 8 runs with.
   
   1. Adding the EasyOutOfMemory annotation to TestValueVector.java.
   2. Bumping the directMemoryMb value in pom.xml. We set up a swap file at the start of the CI run now so you could try to taking this up to 3Gb.
   
   


-- 
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


Re: [PR] DRILL-8467: Update Netty to 4.1.101 (drill)

Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on PR #2857:
URL: https://github.com/apache/drill/pull/2857#issuecomment-1872950972

   Update on my comments above: the direct memory limit increase has just been merged to master. Please rebase this and let's see what we get.


-- 
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


Re: [PR] Update Netty to 4.1.101 (drill)

Posted by "rymarm (via GitHub)" <gi...@apache.org>.
rymarm commented on PR #2857:
URL: https://github.com/apache/drill/pull/2857#issuecomment-1860632954

   @cgivre I don't think that we need to add config options for changing the default chunk size and allocation cache enabling, because both are fundamental for Drill and I don't think there are unique cases when Drill works better with other than default values so a user wishes to change them.
   
   But even If a user wishes to change them for some reason, he still can use java options `-Dio.netty.allocator.useCacheForAllThreads ` and `-Dio.netty.allocator.maxOrder` to change the values.


-- 
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


Re: [PR] Update Netty to 4.1.101 (drill)

Posted by "rymarm (via GitHub)" <gi...@apache.org>.
rymarm commented on PR #2857:
URL: https://github.com/apache/drill/pull/2857#issuecomment-1860638766

   I need to find out now, why Drill tests on Java 8 failed with our of memory error:
   ```
   Errors:   
   [ERROR]   TestValueVector.testFixedVectorReallocation »  Unexpected exception, expected<org.apache.drill.exec.exception.OversizedAllocationException> but was<org.apache.drill.exec.exception.OutOfMemoryException>   
   [ERROR]   TestValueVector.testVariableVectorReallocation »  Unexpected exception, expected<org.apache.drill.exec.exception.OversizedAllocationException> but was<org.apache.drill.exec.exception.OutOfMemoryException>
   ```


-- 
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


Re: [PR] DRILL-8467: Update Netty to 4.1.101 (drill)

Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on PR #2857:
URL: https://github.com/apache/drill/pull/2857#issuecomment-1872747437

   P.S. the failing test class is fine on my laptop under IBM JDK 8 so I do think we can fix this in the CI with some environment tweaks.


-- 
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


Re: [PR] DRILL-8467: Update Netty to 4.1.101 (drill)

Posted by "rymarm (via GitHub)" <gi...@apache.org>.
rymarm commented on PR #2857:
URL: https://github.com/apache/drill/pull/2857#issuecomment-1872957731

   @jnturton Thank you so much for the investigation! I didn't have time to check tests failures.
   P.S. You decide to finish all unresolved tasks until the end of the year?)


-- 
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