Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16856 )
Change subject: IMPALA-10375: Lock down which filesystems use the file handle cache ...................................................................... Patch Set 1: (1 comment) I suggested a refactoring, it would make the patch a bit bigger but might pay off. LMK what you think. http://gerrit.cloudera.org:8080/#/c/16856/1/be/src/runtime/io/scan-range.cc File be/src/runtime/io/scan-range.cc: http://gerrit.cloudera.org:8080/#/c/16856/1/be/src/runtime/io/scan-range.cc@200 PS1, Line 200: if (is_file_handle_caching_enabled() && filesystem_supports_handle_caching(file()) && I was originally thinking about this from the point of view of performance overhead and mostly convinced myself that the extra string parsing wouldn't add much. But it occurs to me that the code would probably be simplified overall if we figured out the filesystem implementation once per scan range, i.e. at the same time as AssignDiskQueue() is called. We could convert it to an enum like FsType in the frontend these disk_id/filename checks in ScanRange would be a bit less ad-hoc. I think that would make handling https://issues.apache.org/jira/browse/HDFS-15289 simpler too once we get to that. -- To view, visit http://gerrit.cloudera.org:8080/16856 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I136c3da9d19590cdbe8623d22480b8dd07192ce3 Gerrit-Change-Number: 16856 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 11 Dec 2020 05:16:48 +0000 Gerrit-HasComments: Yes
