Skip to content

Commit

Permalink
lsa: Fix misaccunting of used space when allocating lsa_buffers
Browse files Browse the repository at this point in the history
lsa_buffer allocations are aligned to 4K. If smaller size is
requested, whole 4K is used. However, only requested size was used in
accounting segment occupancy. This can confuse reclaimer which may
think the segment is sparse while it is actually dense, and compacting
it will yield no or little gain. This can cause inefficient memory
reclamation or lack of progress.

Refs #9038
Message-Id: <20210720104110.463812-1-tgrabiec@scylladb.com>
  • Loading branch information
tgrabiec authored and avikivity committed Jul 20, 2021
1 parent d2b53bc commit 50ec3ea
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 3 deletions.
33 changes: 33 additions & 0 deletions test/boost/logalloc_test.cc
Expand Up @@ -44,6 +44,7 @@
#include "utils/logalloc.hh"
#include "utils/managed_ref.hh"
#include "utils/managed_bytes.hh"
#include "utils/chunked_vector.hh"
#include "test/lib/log.hh"
#include "log.hh"
#include "test/lib/random_utils.hh"
Expand Down Expand Up @@ -1887,4 +1888,36 @@ SEASTAR_THREAD_TEST_CASE(test_weak_ptr) {
BOOST_REQUIRE(!obj_wptr);
}

SEASTAR_THREAD_TEST_CASE(test_buf_alloc_compaction) {
logalloc::region region;
size_t buf_size = 128; // much smaller than region_impl::buf_align

utils::chunked_vector<lsa_buffer> bufs;

bool reclaimer_run = false;
region.make_evictable([&] {
reclaimer_run = true;
if (bufs.empty()) {
return memory::reclaiming_result::reclaimed_nothing;
}
bufs.pop_back();
return memory::reclaiming_result::reclaimed_something;
});

allocating_section as;
while (!reclaimer_run) {
as(region, [&] {
bufs.emplace_back(region.alloc_buf(buf_size));
});
}

// Allocate a few segments more after eviction starts
// to make sure we can really make forward progress.
for (int i = 0; i < 32*100; ++i) {
as(region, [&] {
bufs.emplace_back(region.alloc_buf(buf_size));
});
}
}

#endif
8 changes: 5 additions & 3 deletions utils/logalloc.cc
Expand Up @@ -1400,8 +1400,9 @@ class region_impl final : public basic_region_impl {
segment_descriptor& desc = shard_segment_pool.descriptor(_buf_active);
ptr._desc = &desc;
desc._buf_pointers.emplace_back(entangled::make_paired_with(ptr._link));
desc.record_alloc(buf_size);
_buf_active_offset += align_up(buf_size, buf_align);
auto alloc_size = align_up(buf_size, buf_align);
desc.record_alloc(alloc_size);
_buf_active_offset += alloc_size;

return ptr;
}
Expand All @@ -1414,7 +1415,8 @@ class region_impl final : public basic_region_impl {
_closed_occupancy -= seg->occupancy();
}

desc.record_free(buf._size);
auto alloc_size = align_up(buf._size, buf_align);
desc.record_free(alloc_size);
poison(buf._buf, buf._size);

// Pack links so that segment compaction only has to walk live objects.
Expand Down

0 comments on commit 50ec3ea

Please sign in to comment.