Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/9576 )
Change subject: IMPALA-6638: Reduce file handle cache lock contention ...................................................................... Patch Set 1: (1 comment) > (1 comment) > > This change can increase the number of parallel calls to the > namenode by FileHandleCache, which was limited by the number of > partitions before. I do not know if this can cause problems or not, > but I would prefer to mention it as a possible side effect, or > limit the number of parallel hdfsOpenFile calls somehow. Good point. This code executes in the disk IO threads, and there are a limited number of disk IO threads (1/spinning disk, 8/SSD, etc). This can be larger than the number of partitions for the cache, but it is limited. The reason that I consider it ok to increase the parallelism is that we already use this level of parallelism when the file handle cache is off. When the cache is off, we use DiskIoMgr::GetExclusiveHdfsFileHandle(), which doesn't need to get any lock. This is fixing up the file handle cache so that it can be as fast as when the cache is off. http://gerrit.cloudera.org:8080/#/c/9576/1/be/src/runtime/io/handle-cache.inline.h File be/src/runtime/io/handle-cache.inline.h: http://gerrit.cloudera.org:8080/#/c/9576/1/be/src/runtime/io/handle-cache.inline.h@129 PS1, Line 129: pair<typename MapType::iterator, typename MapType::iterator> range = : p.cache.equal_range(*fname); : FileHandleEntry entry(new_fh, p.lru_list); : typename MapType::iterator new_it = p.cache.emplace_hint(range.second, : *fname, std::move(entry)); > Does the hint logic make a difference anymore? I think that it would be bet Good point, that is simpler/better. Done. -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Comment-Date: Mon, 12 Mar 2018 17:24:28 +0000 Gerrit-HasComments: Yes