Fix memory tracking with min_bytes_to_use_mmap_io

With min_bytes_to_use_mmap_io=1 memory regions for mmap() will not be
accounted. But this creates issues, since if you will run simple
"SELECT * FROM big_enough_table" it will mmap lot's of files and the
process RSS will grow at some point, and before the kernel will
understand that it needs to unmap physical pages the clickhouse-server
will sync total_memory_tracker with RSS of the process and the
allocations will fail by per-server memory limits.

Also note, that this does not address all the issues with mmap()
regions.

v2: move track/untrack into the MMapReadBufferFromFileWithCache to avoid
data-race [1]

  [1]: https://clickhouse-test-reports.s3.yandex.net/23211/6acf6cb9b22cee7db8d1aa523a6208f63e792f7a/functional_stateful_tests_(thread).html#fail1
This commit is contained in:
Azat Khuzhin 2021-04-17 08:38:00 +03:00
parent 3f3f928c1f
commit 6064f3ef1d
5 changed files with 33 additions and 0 deletions

View File

@ -1,4 +1,5 @@
#include <IO/MMapReadBufferFromFileWithCache.h>
#include <Common/CurrentMemoryTracker.h>
namespace DB
@ -74,4 +75,16 @@ off_t MMapReadBufferFromFileWithCache::seek(off_t offset, int whence)
return new_pos;
}
void MMapReadBufferFromFileWithCache::track(size_t bytes_)
{
CurrentMemoryTracker::alloc(bytes_);
tracked_bytes = bytes_;
}
MMapReadBufferFromFileWithCache::~MMapReadBufferFromFileWithCache()
{
if (tracked_bytes)
CurrentMemoryTracker::free(tracked_bytes);
}
}

View File

@ -16,13 +16,27 @@ public:
/// Map till end of file.
MMapReadBufferFromFileWithCache(MMappedFileCache & cache, const std::string & file_name, size_t offset);
~MMapReadBufferFromFileWithCache() override;
off_t getPosition() override;
std::string getFileName() const override;
off_t seek(off_t offset, int whence) override;
/// Track memory with MemoryTracker.
///
/// NOTE: that this is not the size of mmap() region, but estimated size of
/// data that will be read (see MergeTreeReaderStream).
/// And from one point of view it should not be accounted here, since the
/// kernel may unmap physical pages, but in practice firstly it will mmap it,
/// RSS will grow, total_memory_tracker will be synced with RSS and the
/// allocations will start to fail.
void track(size_t bytes_);
private:
MMappedFileCache::MappedPtr mapped;
size_t tracked_bytes = 0;
void init();
};

View File

@ -50,6 +50,7 @@ std::unique_ptr<ReadBufferFromFileBase> createReadBufferFromFileBase(
try
{
auto res = std::make_unique<MMapReadBufferFromFileWithCache>(*mmap_cache, filename_, 0);
res->track(estimated_size);
ProfileEvents::increment(ProfileEvents::CreatedReadBufferMMap);
return res;
}

View File

@ -0,0 +1,5 @@
drop table if exists data_01818;
create table data_01818 (key Int, value String) engine=MergeTree() order by key settings min_bytes_for_wide_part=0 as select number, randomPrintableASCII(100) from numbers(1e6);
select * from data_01818 format Null settings min_bytes_to_use_mmap_io=1, max_memory_usage='20Mi', max_threads=1; -- { serverError 241 }
select * from data_01818 format Null settings min_bytes_to_use_mmap_io=1e9, max_memory_usage='20Mi', max_threads=1;