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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to