impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5352: Age out unused file handles from the cache
Date Wed, 16 Aug 2017 16:47:46 GMT
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5352: Age out unused file handles from the cache
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7640/1/be/src/runtime/disk-io-mgr-handle-cache.h
File be/src/runtime/disk-io-mgr-handle-cache.h:

PS1, Line 98: Status
comment on when this returns an error


PS1, Line 126:   struct LruListEntry {
             :     LruListEntry(typename MapType::iterator map_entry_in);
             :     typename MapType::iterator map_entry;
             :     uint64_t timestamp_seconds;
             :   };
It'd be nice to have a validate() method to check internal consistency of these structures
that reference each other.


PS1, Line 180: Maximum time that a file handle can be unused before
Maximum time before an unused file handle is aged out of the cache.


http://gerrit.cloudera.org:8080/#/c/7640/1/be/src/runtime/disk-io-mgr-handle-cache.inline.h
File be/src/runtime/disk-io-mgr-handle-cache.inline.h:

PS1, Line 73: Status 
Might as well be a void fn


Line 78:       &FileHandleCache<NUM_PARTITIONS>::EvictHandlesLoop, this));
nit: 4spaces


http://gerrit.cloudera.org:8080/#/c/7640/1/tests/custom_cluster/test_hdfs_fd_caching.py
File tests/custom_cluster/test_hdfs_fd_caching.py:

PS1, Line 137: unused_file_handle_timeout_mins
might be better to take the timeout in seconds, then you can make the timeout even lower for
tests and just keep seconds consistently in the code


-- 
To view, visit http://gerrit.cloudera.org:8080/7640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefe04b3e2e22123ecb8b3e494934c93dfb29682e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message