diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index a3fc00a6a2..804a8a6314 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -313,6 +313,9 @@ Branch-2 ( Unreleased changes ) HDFS-799. libhdfs must call DetachCurrentThread when a thread is destroyed. (Colin Patrick McCabe via eli) + HDFS-3306. fuse_dfs: don't lock release operations. + (Colin Patrick McCabe via eli) + OPTIMIZATIONS HDFS-2982. Startup performance suffers when there are many edit log diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/contrib/fuse-dfs/src/fuse_impls_release.c b/hadoop-hdfs-project/hadoop-hdfs/src/contrib/fuse-dfs/src/fuse_impls_release.c index 78ea2b1c1a..2019ff528e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/contrib/fuse-dfs/src/fuse_impls_release.c +++ b/hadoop-hdfs-project/hadoop-hdfs/src/contrib/fuse-dfs/src/fuse_impls_release.c @@ -22,10 +22,17 @@ #include "fuse_connect.h" /** - * This mutex is to protect releasing a file handle in case the user calls close in different threads - * and fuse passes these calls to here. + * release a fuse_file_info structure. + * + * When this function is invoked, there are no more references to our + * fuse_file_info structure that exist anywhere. So there is no need for + * locking to protect this structure here. + * + * Another thread could open() the same file, and get a separate, different file + * descriptor with a different, separate fuse_file_info structure. In HDFS, + * this results in one writer winning and overwriting everything the other + * writer has done. */ -pthread_mutex_t release_mutex = PTHREAD_MUTEX_INITIALIZER; int dfs_release (const char *path, struct fuse_file_info *fi) { TRACE1("release", path) @@ -39,49 +46,21 @@ int dfs_release (const char *path, struct fuse_file_info *fi) { assert('/' == *path); int ret = 0; - - // - // Critical section - protect from multiple close calls in different threads. - // (no returns until end) - // - - pthread_mutex_lock(&release_mutex); - - if (NULL != (void*)fi->fh) { - - dfs_fh *fh = (dfs_fh*)fi->fh; - assert(fh); - - hdfsFile file_handle = (hdfsFile)fh->hdfsFH; - - if (NULL != file_handle) { - if (hdfsCloseFile(fh->fs, file_handle) != 0) { - ERROR("Could not close handle %ld for %s\n",(long)file_handle, path); - ret = -EIO; - } - } - - if (fh->buf != NULL) { - free(fh->buf); - } - - if (doDisconnect(fh->fs)) { + dfs_fh *fh = (dfs_fh*)fi->fh; + assert(fh); + hdfsFile file_handle = (hdfsFile)fh->hdfsFH; + if (NULL != file_handle) { + if (hdfsCloseFile(fh->fs, file_handle) != 0) { + ERROR("Could not close handle %ld for %s\n",(long)file_handle, path); ret = -EIO; } - - // this is always created and initialized, so always destroy it. (see dfs_open) - pthread_mutex_destroy(&fh->mutex); - - free(fh); - - fi->fh = (uint64_t)0; } - - pthread_mutex_unlock(&release_mutex); - - // - // End critical section - // - + free(fh->buf); + if (doDisconnect(fh->fs)) { + ret = -EIO; + } + pthread_mutex_destroy(&fh->mutex); + free(fh); + fi->fh = 0; return ret; }