Merge pull request #49733 from ClickHouse/nickitat-patch-11

Do not allocate own buffer in CachedOnDiskReadBufferFromFile when `use_external_buffer == true`
This commit is contained in:
Kseniia Sumarokova 2023-05-12 00:05:25 +02:00 committed by GitHub
commit f5b959624d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -50,7 +50,7 @@ CachedOnDiskReadBufferFromFile::CachedOnDiskReadBufferFromFile(
bool use_external_buffer_, bool use_external_buffer_,
std::optional<size_t> read_until_position_, std::optional<size_t> read_until_position_,
std::shared_ptr<FilesystemCacheLog> cache_log_) std::shared_ptr<FilesystemCacheLog> cache_log_)
: ReadBufferFromFileBase(settings_.remote_fs_buffer_size, nullptr, 0, file_size_) : ReadBufferFromFileBase(use_external_buffer_ ? 0 : settings_.remote_fs_buffer_size, nullptr, 0, file_size_)
#ifndef NDEBUG #ifndef NDEBUG
, log(&Poco::Logger::get("CachedOnDiskReadBufferFromFile(" + source_file_path_ + ")")) , log(&Poco::Logger::get("CachedOnDiskReadBufferFromFile(" + source_file_path_ + ")"))
#else #else
@ -151,10 +151,8 @@ CachedOnDiskReadBufferFromFile::getCacheReadBuffer(const FileSegment & file_segm
/// Do not allow to use asynchronous version of LocalFSReadMethod. /// Do not allow to use asynchronous version of LocalFSReadMethod.
local_read_settings.local_fs_method = LocalFSReadMethod::pread; local_read_settings.local_fs_method = LocalFSReadMethod::pread;
// The buffer will unnecessarily allocate a Memory of size local_fs_buffer_size, which will then if (use_external_buffer)
// most likely be unused because we're swap()ping our own internal_buffer into local_read_settings.local_fs_buffer_size = 0;
// implementation_buffer before each read. But we can't just set local_fs_buffer_size = 0 here
// because some buffer implementations actually use that memory (e.g. for prefetching).
auto buf = createReadBufferFromFileBase(path, local_read_settings); auto buf = createReadBufferFromFileBase(path, local_read_settings);