Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11444 )

Change subject: IMPALA-7556: part 1: handle different file systems via 
polymorphism
......................................................................


Patch Set 1:

(11 comments)

This reply contains comments for PS1 that tell you where the code was copied 
from.

http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.h
File be/src/runtime/io/hdfs-file-reader.h:

http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.h@46
PS1, Line 46:   /// Hadoop filesystem that contains the file being read.
            :   hdfsFS hdfs_fs_;
Same as 'fs_' in scan_range_.


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.h@49
PS1, Line 49:   /// The hdfs file handle is stored here in three cases:
            :   /// 1. The file handle cache is off (max_cached_file_handles == 
0).
            :   /// 2. The scan range is using hdfs caching.
            :   /// -OR-
            :   /// 3. The hdfs file is expected to be remote (expected_local_ 
== false)
            :   /// In each case, the scan range gets a new 
ExclusiveHdfsFileHandle at Open(),
            :   /// owns it exclusively, and destroys it in Close().
            :   ExclusiveHdfsFileHandle* exclusive_hdfs_fh_ = nullptr;
            :
            :   /// If true, we expect the reads to be a local read. Note that 
if this is false,
            :   /// it does not necessarily mean we expect the read to be 
remote, and that we never
            :   /// create scan ranges where some of the range is expected to 
be remote and some of it
            :   /// local.
            :   /// TODO: we can do more with this
            :   const bool expected_local_;
            :
            :   /// Total number of bytes read remotely. This is necessary to 
maintain a count of
            :   /// the number of remote scan ranges. Since IO statistics can 
be collected multiple
            :   /// times for a scan range, it is necessary to keep some state 
about whether this
            :   /// scan range has already been counted as remote. There is 
also a requirement to
            :   /// log the number of unexpected remote bytes for a scan range. 
To solve both
            :   /// requirements, maintain num_remote_bytes_ on the ScanRange 
and push it to the
            :   /// reader_ once at the close of the scan range.
            :   int64_t num_remote_bytes_ = 0;
Extracted from ScanRange


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc
File be/src/runtime/io/hdfs-file-reader.cc:

http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc@45
PS1, Line 45: Status HdfsFileReader::Open(bool use_file_handle_cache) {
Copied from ScanRange::Open()


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc@74
PS1, Line 74: Status HdfsFileReader::ReadFromPos(int64_t file_offset, uint8_t* 
buffer,
Copied from ScanRange::Read()


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc@201
PS1, Line 201: Status HdfsFileReader::Close() {
Copied from ScanRange::Close()


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc@249
PS1, Line 249: Status HdfsFileReader::ReadFromCache(const unique_lock<mutex>& 
reader_lock,
Copied from ScanRange::ReadFromCache()


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc@310
PS1, Line 310: void HdfsFileReader::GetHdfsStatistics(hdfsFile hdfs_file) {
Copied from ScanRange::GetHdfsStatistics()


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/local-file-reader.h
File be/src/runtime/io/local-file-reader.h:

http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/local-file-reader.h@40
PS1, Line 40:   FILE* file_ = nullptr;
Extracted from ScanRange


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/local-file-reader.cc
File be/src/runtime/io/local-file-reader.cc:

http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/local-file-reader.cc@36
PS1, Line 36: Status LocalFileReader::Open(bool use_file_handle_cache) {
Copied from ScanRange::Open()


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/local-file-reader.cc@53
PS1, Line 53: Status LocalFileReader::ReadFromPos(int64_t file_offset, uint8_t* 
buffer,
Copied from ScanRange::Read()


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/local-file-reader.cc@107
PS1, Line 107: Status LocalFileReader::Close() {
Copied from ScanRange::Close()



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82
Gerrit-Change-Number: 11444
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Thu, 20 Sep 2018 11:46:42 +0000
Gerrit-HasComments: Yes

Reply via email to