[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 7 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 24 Feb 2021 00:23:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. IMPALA-10504: Add tracing for remote block reads This patch logs metadata for the first unexpected remote read of each scanrange when the flag fs_trace_remote_reads is set to true. This logging is intended to help diagnose the root cause of remote reads. Since a message may be logged for each scan range, there could be several hundred lines of output in a degenerate case. However, the remote read condition is not expected and verbose output may be needed to diagnose the root cause. Reviewed-by: Aman Sinha Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Reviewed-on: http://gerrit.cloudera.org:8080/17062 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/runtime/io/hdfs-file-reader.cc M be/src/runtime/io/hdfs-file-reader.h 2 files changed, 44 insertions(+), 0 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 8 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 7 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Tue, 23 Feb 2021 18:38:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6915/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 7 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Tue, 23 Feb 2021 18:38:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 6: Code-Review+2 Looks good. Thanks for adding this! -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 6 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Tue, 23 Feb 2021 16:26:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8201/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 6 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Tue, 23 Feb 2021 04:25:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Hello Aman Sinha, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17062 to look at the new patch set (#6). Change subject: IMPALA-10504: Add tracing for remote block reads .. IMPALA-10504: Add tracing for remote block reads This patch logs metadata for the first unexpected remote read of each scanrange when the flag fs_trace_remote_reads is set to true. This logging is intended to help diagnose the root cause of remote reads. Since a message may be logged for each scan range, there could be several hundred lines of output in a degenerate case. However, the remote read condition is not expected and verbose output may be needed to diagnose the root cause. Reviewed-by: Aman Sinha Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 --- M be/src/runtime/io/hdfs-file-reader.cc M be/src/runtime/io/hdfs-file-reader.h 2 files changed, 44 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/17062/6 -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 6 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 5: This is looking good to me. Please wrap the long lines, then I'll bump to +2. -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 5 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Mon, 22 Feb 2021 18:54:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 5 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Mon, 22 Feb 2021 18:54:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8183/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 5 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Sun, 21 Feb 2021 21:24:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/17062 )
Change subject: IMPALA-10504: Add tracing for remote block reads
..
Patch Set 5:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/17062/5/be/src/runtime/io/hdfs-file-reader.h
File be/src/runtime/io/hdfs-file-reader.h:
http://gerrit.cloudera.org:8080/#/c/17062/5/be/src/runtime/io/hdfs-file-reader.h@81
PS5, Line 81: /// Return a string that contains the block indexes and list of
hosts where each block resides
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/17062/5/be/src/runtime/io/hdfs-file-reader.cc
File be/src/runtime/io/hdfs-file-reader.cc:
http://gerrit.cloudera.org:8080/#/c/17062/5/be/src/runtime/io/hdfs-file-reader.cc@84
PS5, Line 84: std::string HdfsFileReader::GetHostList(int64_t file_offset,
int64_t bytes_to_read) const {
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/17062/5/be/src/runtime/io/hdfs-file-reader.cc@85
PS5, Line 85: char*** hosts = hdfsGetHosts(hdfs_fs_,
scan_range_->file_string()->c_str(), file_offset, bytes_to_read);
line too long (106 > 90)
http://gerrit.cloudera.org:8080/#/c/17062/5/be/src/runtime/io/hdfs-file-reader.cc@240
PS5, Line 240: if (FLAGS_fs_trace_remote_reads && expected_local_ &&
num_remote_bytes_ > 0 && is_first_read) {
line too long (101 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/17062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257
Gerrit-Change-Number: 17062
Gerrit-PatchSet: 5
Gerrit-Owner: Kurt Deschler
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Comment-Date: Sun, 21 Feb 2021 21:02:31 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Hello Aman Sinha, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17062 to look at the new patch set (#5). Change subject: IMPALA-10504: Add tracing for remote block reads .. IMPALA-10504: Add tracing for remote block reads This patch logs metadata for the first unexpected remote read of each scanrange when the flag fs_trace_remote_reads is set to true. This logging is intended to help diagnose the root cause of remote reads. Since a message may be logged for each scan range, there could be several hundred lines of output in a degenerate case. However, the remote read condition is not expected and verbose output may be needed to diagnose the root cause. Reviewed-by: Aman Sinha Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 --- M be/src/runtime/io/hdfs-file-reader.cc M be/src/runtime/io/hdfs-file-reader.h 2 files changed, 41 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/17062/5 -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 5 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 4: (2 comments) Thanks for fixing the last round of comments. Code-wise, this is close. We've had issues with logging verbosity in the past, and we may want to limit this or have it off by default. http://gerrit.cloudera.org:8080/#/c/17062/4/be/src/runtime/io/hdfs-file-reader.h File be/src/runtime/io/hdfs-file-reader.h: http://gerrit.cloudera.org:8080/#/c/17062/4/be/src/runtime/io/hdfs-file-reader.h@82 PS4, Line 82: vb1540.halxg.cloudera.com Nit: we would rather not have cloudera.com hostnames in the source code. http://gerrit.cloudera.org:8080/#/c/17062/4/be/src/runtime/io/hdfs-file-reader.cc File be/src/runtime/io/hdfs-file-reader.cc: http://gerrit.cloudera.org:8080/#/c/17062/4/be/src/runtime/io/hdfs-file-reader.cc@239 PS4, Line 239: LOG(INFO) : << "First remote read of scan range on file " : << *scan_range_->file_string() : << " " << num_remote_bytes_ : << " bytes. offsets " << file_offset : << "-" << file_offset+bytes_to_read-1 : << GetHostList(file_offset, bytes_to_read); The only remaining thought is that we may want a way to throttle or disable this. If we are reading parquet, the columns are usually on the same HDFS block, so if one column is hitting it, the others may also hit it. Does this need to be enabled by default? Could it be enabled when someone sees a problem? One option is to add a startup flag to control whether to produce this output. On startup, there can be some logic to turn it on for configurations where it is especially useful. -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 4 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 17 Feb 2021 23:24:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8147/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 4 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 17 Feb 2021 18:26:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/17062 )
Change subject: IMPALA-10504: Add tracing for remote block reads
..
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/17062/4/be/src/runtime/io/hdfs-file-reader.h
File be/src/runtime/io/hdfs-file-reader.h:
http://gerrit.cloudera.org:8080/#/c/17062/4/be/src/runtime/io/hdfs-file-reader.h@81
PS4, Line 81: /// Return a string that contains the block indexes and list of
hosts where each block resides
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/17062/4/be/src/runtime/io/hdfs-file-reader.h@82
PS4, Line 82: /// i.e. [0] { vb1540.halxg.cloudera.com,
vb1541.halxg.cloudera.com, vb1539.halxg.cloudera.com }
line too long (98 > 90)
http://gerrit.cloudera.org:8080/#/c/17062/4/be/src/runtime/io/hdfs-file-reader.cc
File be/src/runtime/io/hdfs-file-reader.cc:
http://gerrit.cloudera.org:8080/#/c/17062/4/be/src/runtime/io/hdfs-file-reader.cc@81
PS4, Line 81: std::string HdfsFileReader::GetHostList(int64_t file_offset,
int64_t bytes_to_read) const {
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/17062/4/be/src/runtime/io/hdfs-file-reader.cc@82
PS4, Line 82: char*** hosts = hdfsGetHosts(hdfs_fs_,
scan_range_->file_string()->c_str(), file_offset, bytes_to_read);
line too long (106 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/17062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257
Gerrit-Change-Number: 17062
Gerrit-PatchSet: 4
Gerrit-Owner: Kurt Deschler
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Comment-Date: Wed, 17 Feb 2021 18:06:51 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Hello Aman Sinha, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17062 to look at the new patch set (#4). Change subject: IMPALA-10504: Add tracing for remote block reads .. IMPALA-10504: Add tracing for remote block reads This patch logs metadata for the first unexpected remote read of each scanrange. This logging is intended to help diagnose the root cause of remote reads. Since a message may be logged for each scan range, there could be several hundred lines of output in a degenerate case. However, the remote read condition is not expected and verbose output may be needed to diagnose the root cause. Reviewed-by: Aman Sinha Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 --- M be/src/runtime/io/hdfs-file-reader.cc M be/src/runtime/io/hdfs-file-reader.h 2 files changed, 38 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/17062/4 -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 4 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Joe McDonnell has posted comments on this change. (
http://gerrit.cloudera.org:8080/17062 )
Change subject: IMPALA-10504: Add tracing for remote block reads
..
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/17062/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17062/3//COMMIT_MSG@9
PS3, Line 9: first unexpected remote read of each
: scanrange
Does this happen for large numbers of scan ranges simultaneously or is it more
sporadic?
I'm just trying to get a sense of how much output this could produce.
http://gerrit.cloudera.org:8080/#/c/17062/3//COMMIT_MSG@13
PS3, Line 13: Reviewed-by: Aman Sinha
Nit: Remove this line (gerrit will add these lines when this merges).
http://gerrit.cloudera.org:8080/#/c/17062/3/be/src/runtime/io/hdfs-file-reader.cc
File be/src/runtime/io/hdfs-file-reader.cc:
http://gerrit.cloudera.org:8080/#/c/17062/3/be/src/runtime/io/hdfs-file-reader.cc@81
PS3, Line 81: appendHostList(const hdfsFS hdfs_fs, const char* file_name,
: int64_t file_offset, int64_t bytes_to_read, ostream& ostr) {
Nit: Couple style things
It may make sense to make this a private method on HdfsFileReader rather than a
regular C-style function. The hdfs_fs and file_name both come from the class
anyway. Either way, it would be nice to have a comment describing the expected
behavior.
For this, I think it would also be cleaner not to have to pass in the ostream.
One way to do it would be to use a std::ostringstream to collect the individual
logging statements and then have this function return a std::string (i.e.
ostreamstream's str() method). Then the logging would look like:
LOG(INFO) << "Normal log stuff"
<< appendHostList(fs, filename, offset, ...);
http://gerrit.cloudera.org:8080/#/c/17062/3/be/src/runtime/io/hdfs-file-reader.cc@232
PS3, Line 232: int64_t save_num_remote_bytes = num_remote_bytes_;
Nit: Since we are using this like a boolean, does it make sense to have a
boolean saying whether we have printed the unexpected remote logging already?
http://gerrit.cloudera.org:8080/#/c/17062/3/be/src/runtime/io/hdfs-file-reader.cc@242
PS3, Line 242: num_remote_bytes_-save_num_remote_bytes
Nit: We only enter this if save_num_remote_bytes==0, so we could drop this.
--
To view, visit http://gerrit.cloudera.org:8080/17062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257
Gerrit-Change-Number: 17062
Gerrit-PatchSet: 3
Gerrit-Owner: Kurt Deschler
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Comment-Date: Wed, 17 Feb 2021 01:38:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8131/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 3 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 12 Feb 2021 18:11:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Hello Aman Sinha, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17062 to look at the new patch set (#3). Change subject: IMPALA-10504: Add tracing for remote block reads .. IMPALA-10504: Add tracing for remote block reads This patch logs metadata for the first unexpected remote read of each scanrange. This logging is intended to help diagnose the root cause of remote reads. Reviewed-by: Aman Sinha Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 --- M be/src/runtime/io/hdfs-file-reader.cc 1 file changed, 32 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/17062/3 -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 3 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8130/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 2 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 12 Feb 2021 17:47:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17062 ) Change subject: IMPALA-10504: Add tracing for remote block reads .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8129/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 1 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 12 Feb 2021 17:45:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/17062 )
Change subject: IMPALA-10504: Add tracing for remote block reads
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17062/2/be/src/runtime/io/hdfs-file-reader.cc
File be/src/runtime/io/hdfs-file-reader.cc:
http://gerrit.cloudera.org:8080/#/c/17062/2/be/src/runtime/io/hdfs-file-reader.cc@81
PS2, Line 81: void appendHostList(const hdfsFS hdfs_fs, const char* file_name,
int64_t file_offset, int64_t bytes_to_read, ostream& ostr) {
line too long (125 > 90)
http://gerrit.cloudera.org:8080/#/c/17062/2/be/src/runtime/io/hdfs-file-reader.cc@236
PS2, Line 236: appendHostList(hdfs_fs_,
scan_range_->file_string()->c_str(), file_offset, bytes_to_read,
line too long (97 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/17062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257
Gerrit-Change-Number: 17062
Gerrit-PatchSet: 2
Gerrit-Owner: Kurt Deschler
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 12 Feb 2021 17:29:08 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Hello Aman Sinha, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17062 to look at the new patch set (#2). Change subject: IMPALA-10504: Add tracing for remote block reads .. IMPALA-10504: Add tracing for remote block reads This patch logs metadata for the first unexpected remote read of each scanrange. This logging is intended to help diagnose the root cause of remote reads. Reviewed-by: Aman Sinha Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 --- M be/src/runtime/io/hdfs-file-reader.cc 1 file changed, 30 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/17062/2 -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 2 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/17062 )
Change subject: IMPALA-10504: Add tracing for remote block reads
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17062/1/be/src/runtime/io/hdfs-file-reader.cc
File be/src/runtime/io/hdfs-file-reader.cc:
http://gerrit.cloudera.org:8080/#/c/17062/1/be/src/runtime/io/hdfs-file-reader.cc@81
PS1, Line 81: void appendHostList(const hdfsFS hdfs_fs, const char* file_name,
int64_t file_offset, int64_t bytes_to_read, ostream& ostr) {
line too long (125 > 90)
http://gerrit.cloudera.org:8080/#/c/17062/1/be/src/runtime/io/hdfs-file-reader.cc@236
PS1, Line 236: appendHostList(hdfs_fs_,
scan_range_->file_string()->c_str(), file_offset, bytes_to_read,
line too long (97 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/17062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257
Gerrit-Change-Number: 17062
Gerrit-PatchSet: 1
Gerrit-Owner: Kurt Deschler
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 12 Feb 2021 17:27:21 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Hello Aman Sinha, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/17062 to review the following change. Change subject: IMPALA-10504: Add tracing for remote block reads .. IMPALA-10504: Add tracing for remote block reads This patch logs metadata for the first unexpected remote read of each scanrange. This logging is intended to help diagnose the root cause of remote reads. Reviewed-by: Aman Sinha Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 --- M be/src/runtime/io/hdfs-file-reader.cc 1 file changed, 30 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/17062/1 -- To view, visit http://gerrit.cloudera.org:8080/17062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257 Gerrit-Change-Number: 17062 Gerrit-PatchSet: 1 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Aman Sinha
