impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joe McDonnell (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4623: Enable file handle cache
Date Wed, 10 May 2017 20:27:51 GMT
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 <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message