Joe McDonnell has posted comments on this change. Change subject: IMPALA-4623: Enable file handle cache ......................................................................
Patch Set 4: (4 comments) Fixed the test comments and fixed some errors in the code found by other existing tests (custom_cluster/test_hdfs_fd_caching.py tests the metrics). http://gerrit.cloudera.org:8080/#/c/6478/4/be/src/runtime/disk-io-mgr.cc File be/src/runtime/disk-io-mgr.cc: Line 1331: ImpaladMetrics::IO_MGR_NUM_CACHED_FILE_HANDLES->Increment(-1L); > I'm splitting hairs about the exact meaning of cached. I found an existing test that was checking the metrics, and I've changed this code to match what that test was expecting. IO_MGR_NUM_FILE_HANDLES_OUTSTANDING is the number of file handles in use. IO_MGR_NUM_CACHED_FILE_HANDLES now is the count of all files handles. It would also make sense for this to exclude the oustanding file handles, but right now, this counts all file handles. http://gerrit.cloudera.org:8080/#/c/6478/4/tests/query_test/test_hdfs_fd_caching.py File tests/query_test/test_hdfs_fd_caching.py: Line 76: # File deleted: either # rows is the same or # rows = 0 > we don't make any guarantees for this, so might as well not check the resul Changed to not check the results. Line 98: new_table_location = "{0}/test-warehouse/{1}".format(FILESYSTEM_PREFIX, unique_database) > long lines Done Line 105: # Delete the whole directory (including data file) > this function is pretty much identical to the one above (and below), except Done -- To view, visit http://gerrit.cloudera.org:8080/6478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5ff60971dd653c3b6a0e13928cfa9fc59d078d Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
