[ 
https://issues.apache.org/jira/browse/HDFS-6109?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14012594#comment-14012594
 ] 

stack commented on HDFS-6109:
-----------------------------

nit: Nothing wrong w/ the below but I had a bit of a visceral reaction to the 
duplicated fat call to syncFileRangeIfPossible.  Any way of making it 'neater' 
(set a boolean if we need to call submitSyncFileRangeRequest?

{code}
+          if (syncBehindWritesInBackground) {
+            FsVolumeSpi volume = this.datanode.getFSDataset().getVolume(block);
+            if (volume instanceof FsVolumeImpl) {
+              FsDatasetAsyncDiskService asyncDiskService = this.datanode
+                  .getFSDataset().getFsDatasetAsyncDiskService();
+              asyncDiskService.submitSyncFileRangeRequest(
+                  (FsVolumeImpl) volume, outFd, lastCacheManagementOffset,
+                  offsetInBlock - lastCacheManagementOffset,
+                  NativeIO.POSIX.SYNC_FILE_RANGE_WRITE);
+            } else {
+              LOG.warn("Fallback to foreground sync_file_range, unexpected 
volume type:"
+                  + volume.getClass().getSimpleName());
+              NativeIO.POSIX.syncFileRangeIfPossible(outFd,
+                  lastCacheManagementOffset, offsetInBlock
+                      - lastCacheManagementOffset,
+                  NativeIO.POSIX.SYNC_FILE_RANGE_WRITE);
+            }
+          } else {
+            NativeIO.POSIX.syncFileRangeIfPossible(outFd,
+                lastCacheManagementOffset,
+                offsetInBlock - lastCacheManagementOffset,
+                NativeIO.POSIX.SYNC_FILE_RANGE_WRITE);
+          }
{code}

Maybe change the class comment on FsDatasetAsyncDiskService now it is no longer 
about deletes only?

Both of above are nits.

Patch lgtm.

> let sync_file_range() system call run in background
> ---------------------------------------------------
>
>                 Key: HDFS-6109
>                 URL: https://issues.apache.org/jira/browse/HDFS-6109
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: datanode
>    Affects Versions: 3.0.0, 2.3.0
>            Reporter: Liang Xie
>            Assignee: Liang Xie
>         Attachments: HDFS-6109-v2.txt, HDFS-6109-v3.txt, HDFS-6109.txt
>
>
> Through we passed SYNC_FILE_RANGE_WRITE to sync_file_range, to make it as 
> asynchronous as possible, it still could be blocked, e.g. the os io request 
> queue is full.
> Since we use sync_file_range just as a page cache advisor role:) it doesn't 
> decide or guarantee the real durability, it would be nice if we could run it 
> in  backgroud. At least my test log showed, a few sync_file_range calls still 
> cost tens of ms or more, due to the happened location is in the critical 
> write path(BlockReceiver class), from a upper view, like HBase application, 
> will "hung" tens of ms as well during Hlog syncing.
> Generally speaking, the patch could not improve too much, but, better than 
> before, right ? :)



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to