Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19228 )
Change subject: IMPALA-11704: (Addendum) fix crash on open for HDFS cache ...................................................................... Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/19228/2/be/src/runtime/io/hdfs-file-reader.cc File be/src/runtime/io/hdfs-file-reader.cc: http://gerrit.cloudera.org:8080/#/c/19228/2/be/src/runtime/io/hdfs-file-reader.cc@60 PS2, Line 60: Status HdfsFileReader::Open(bool use_delayed_open) { : unique_lock<SpinLock> hdfs_lock(lock_); : RETURN_IF_ERROR(scan_range_->cancel_status_); : : if (exclusive_hdfs_fh_ != nullptr) return Status::OK(); : // When using file handle caching or remote data caching, we don't acquire an hdfs file : // handle until we use it to avoid network overhead. : if (use_delayed_open) return Status::OK(); What do you think about removing the argument to Open() and skipping the call when we don't need it? i.e. ReadFromCache() uses an unconditional call and DoReadInternal() only calls it if the open is not delayed. -- To view, visit http://gerrit.cloudera.org:8080/19228 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I741488d6195e586917de220a39090895886a2dc5 Gerrit-Change-Number: 19228 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: David Rorke <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Thu, 10 Nov 2022 18:48:37 +0000 Gerrit-HasComments: Yes
