Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std::bad_alloc may be thrown even though there is evictable memory #4445

Closed
tgrabiec opened this issue Apr 18, 2019 · 0 comments

Comments

Projects
None yet
1 participant
@tgrabiec
Copy link
Contributor

commented Apr 18, 2019

Scylla version: 2.3+ (252c5df)

When we start the LSA reclamation it can be that segment_pool::_free_segments is 0 under some conditions and segment_pool::_current_emergency_reserve_goal is set to 1. The reclamation step is 1 segment, and compact_and_evict_locked() frees 1 segment into the segment_pool. However, segment_pool::reclaim_segments() doesn't free anything to the standard allocator, because the condition _free_segments > _current_emergency_reserve_goal is false. As a result, tracker::impl::reclaim() returns 0 as the amount of released memory, and tracker::reclaim() returns memory::reclaiming_result::reclaimed_nothing, and the seastar allocator thinks it's a real OOM and throws std::bad_alloc.

We should take the reserve goal into account when deciding on how much to release.

@tgrabiec tgrabiec added the bad_alloc label Apr 18, 2019

@tgrabiec tgrabiec self-assigned this Apr 18, 2019

avikivity added a commit that referenced this issue Apr 20, 2019

lsa: Fix potential bad_alloc even though evictable memory exists
When we start the LSA reclamation it can be that
segment_pool::_free_segments is 0 under some conditions and
segment_pool::_current_emergency_reserve_goal is set to 1. The
reclamation step is 1 segment, and compact_and_evict_locked() frees 1
segment back into the segment_pool. However,
segment_pool::reclaim_segments() doesn't free anything to the standard
allocator because the condition _free_segments >
_current_emergency_reserve_goal is false. As a result,
tracker::impl::reclaim() returns 0 as the amount of released memory,
tracker::reclaim() returns
memory::reclaiming_result::reclaimed_nothing and the seastar allocator
thinks it's a real OOM and throws std::bad_alloc.

The fix is to change compact_and_evict() to make sure that reserves
are met, by releasing more if they're not met at entry.

This change also allows us to drop the variant of allocate_segment()
which accepts the reclamation step as a means to refill reserves
faster. This is now not needed, because compact_and_evict() will look
at the reserve deficit to increase the amount of memory to reclaim.

Fixes #4445

Message-Id: <1555671713-16530-1-git-send-email-tgrabiec@scylladb.com>
(cherry picked from commit f092dec)

avikivity added a commit that referenced this issue Apr 23, 2019

lsa: Fix compact_and_evict() being called with a too low step
compact_and_evict gets memory_to_release in bytes while
reclamation step is in segments.

Broken in f092dec.

It doesn't make much difference with the current default step of 1
segment since we cannot reclaim less than that, so shouldn't cause
problems in practice.

Ref #4445

Message-Id: <1556013920-29676-1-git-send-email-tgrabiec@scylladb.com>
(cherry picked from commit 21fbf59)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.