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 <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
