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

Reply via email to