Gabriel39 commented on code in PR #66773:
URL: https://github.com/apache/doris/pull/66773#discussion_r3930923521
##########
be/src/io/fs/hdfs_file_reader.cpp:
##########
@@ -74,25 +74,24 @@ HdfsFileReader::HdfsFileReader(Path path, std::string
fs_name, FileHandleCache::
_accessor(std::move(accessor)),
_mtime(mtime) {
_handle = _accessor.get();
-
- DorisMetrics::instance()->hdfs_file_open_reading->increment(1);
- DorisMetrics::instance()->hdfs_file_reader_total->increment(1);
}
HdfsFileReader::~HdfsFileReader() {
static_cast<void>(close());
}
Status HdfsFileReader::close() {
- bool expected = false;
- if (_closed.compare_exchange_strong(expected, true,
std::memory_order_acq_rel)) {
- DorisMetrics::instance()->hdfs_file_open_reading->increment(-1);
- }
+ _closed = true;
return Status::OK();
}
Status HdfsFileReader::read_at_impl(size_t offset, Slice result, size_t*
bytes_read,
const IOContext* io_ctx) {
+ if (_handle == nullptr) [[unlikely]] {
+ return Status::InternalError("cached hdfs file handle has been
destroyed: {}",
+ _path.native());
+ }
+ RETURN_IF_ERROR(_handle->ensure_open());
Review Comment:
Please check `closed()` before calling `ensure_open()`. A reader constructed
with a known file size and then closed before its first read still reaches this
line, performs `hdfsOpenFile`, and only afterward returns `read closed file`
from `do_read_at_impl()`. This preserves an unnecessary NameNode RPC that the
lazy-open change is intended to avoid. Please move the closed/null guards
before `ensure_open()` and add a close-before-first-read regression test that
verifies the open hook is not invoked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
