[ 
https://issues.apache.org/jira/browse/HIVE-24524?focusedWorklogId=581677&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-581677
 ]

ASF GitHub Bot logged work on HIVE-24524:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 13/Apr/21 10:31
            Start Date: 13/Apr/21 10:31
    Worklog Time Spent: 10m 
      Work Description: abstractdog commented on a change in pull request #1778:
URL: https://github.com/apache/hive/pull/1778#discussion_r612326445



##########
File path: 
llap-server/src/java/org/apache/hadoop/hive/llap/shufflehandler/FadvisedFileRegion.java
##########
@@ -124,39 +149,33 @@ long customShuffleTransfer(WritableByteChannel target, 
long position)
         position += trans; 
         trans = 0;
       }
-      
+
       //write data to the target
       while(byteBuffer.hasRemaining()) {
         target.write(byteBuffer);
       }
       
       byteBuffer.clear();
     }
-    
+
     return actualCount - trans;
   }
 
-  
-  @Override
-  public void releaseExternalResources() {
-    if (readaheadRequest != null) {
-      readaheadRequest.cancel();
-    }
-    super.releaseExternalResources();
-  }
-  
   /**
    * Call when the transfer completes successfully so we can advise the OS that
    * we don't need the region to be cached anymore.
    */
   public void transferSuccessful() {
-    if (manageOsCache && getCount() > 0) {
+    if (manageOsCache && count() > 0) {
       try {
         if (canEvictAfterTransfer) {
-          LOG.debug("shuffleBufferSize: {}, path: {}", shuffleBufferSize, 
identifier);
-          
NativeIO.POSIX.getCacheManipulator().posixFadviseIfPossible(identifier,
-              fd, getPosition(), getCount(),
-              NativeIO.POSIX.POSIX_FADV_DONTNEED);
+          if (fd.valid()) {

Review comment:
       hm, thought this over again, fd.valid() change was needed while I 
haven't been handling deallocate() stuff properly, but now, at this point fd 
should be valid...initially I left this check here because I thought that an 
invalid fd is not a problem (which is true, we won't advise to OS cache, and 
that's it), but as we already have try/catch, we don't need this this check 
(we'll have the exception in the logs 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.

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 581677)
    Time Spent: 1h 10m  (was: 1h)

> LLAP ShuffleHandler: upgrade to netty4
> --------------------------------------
>
>                 Key: HIVE-24524
>                 URL: https://issues.apache.org/jira/browse/HIVE-24524
>             Project: Hive
>          Issue Type: Improvement
>            Reporter: László Bodor
>            Assignee: László Bodor
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> Tez already has a WIP patch for upgrading its shuffle handler to netty4. 
> Netty4 is told to be a possible performance improvement compared to Netty3. 
> However, the refactor is not trivial, TEZ-4157 covers that more or less (the 
> code bases are very similar).
> Background:
> netty4 migration guideline: 
> https://netty.io/wiki/new-and-noteworthy-in-4.0.html
> articles of possible performance improvement:
> https://blog.twitter.com/engineering/en_us/a/2013/netty-4-at-twitter-reduced-gc-overhead.html
> https://developer.squareup.com/blog/upgrading-a-reverse-proxy-from-netty-3-to-4/
> some other notes: Netty3 is EOL since 2016:
> https://netty.io/news/2016/06/29/3-10-6-Final.html



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to