You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Kurt Deschler (Code Review)" <ge...@cloudera.org> on 2021/02/12 17:25:41 UTC
[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 <am...@cloudera.com>
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Feb 2021 23:24:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Feb 2021 04:25:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Feb 2021 16:26:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Sun, 21 Feb 2021 21:02:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Feb 2021 18:06:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Feb 2021 00:23:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 17:29:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 18:11:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Feb 2021 18:38:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 17:47:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Feb 2021 18:54:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Feb 2021 18:26:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Sun, 21 Feb 2021 21:24:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 17:45:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Feb 2021 18:54:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 17:27:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
Change-Id: I8c6a3e92f44813048022edf2b91299b3b0a20257
Reviewed-on: http://gerrit.cloudera.org:8080/17062
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Feb 2021 18:38:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10504: Add tracing for remote block reads
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Feb 2021 01:38:20 +0000
Gerrit-HasComments: Yes