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 <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Comment-Date: Wed, 17 Feb 2021 23:24:15 +0000 Gerrit-HasComments: Yes
