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.

Sign up for GitHub

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

Jump to bottom

[Backport 5.4] Reload reclaimed bloom filters when memory is available #18625

Closed
wants to merge 12 commits into from
Closed

[Backport 5.4] Reload reclaimed bloom filters when memory is available #18625

wants to merge 12 commits into from

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented May 11, 2024

PR #17771 introduced a threshold for the total memory used by all bloom filters across SSTables. When the total usage surpasses the threshold, the largest bloom filter will be removed from memory, bringing the total usage back under the threshold. This PR adds support for reloading such reclaimed bloom filters back into memory when memory becomes available (i.e., within the 10% of available memory earmarked for the reclaimable components).

The SSTables manager now maintains a list of all SSTables whose bloom filter was removed from memory and attempts to reload them when an SSTable, whose bloom filter is still in memory, gets deleted. The manager reloads from the smallest to the largest bloom filter to maximize the number of filters being reloaded into memory.

(cherry picked from commit a53af1f)

(cherry picked from commit 02d272f)

(cherry picked from commit 3ef2f79)

(cherry picked from commit 140d887)

(cherry picked from commit 2340ab6)

(cherry picked from commit 54bb03c)

(cherry picked from commit 69b2a12)

(cherry picked from commit 2406406)

(cherry picked from commit f758d7b)

(cherry picked from commit 0b06119)

(cherry picked from commit a080daa)

(cherry picked from commit 4d22c4b)

Refs #18186

@lkshminarayanan @mergify
sstable: rename local variable in sstable::total_reclaimable_memory_size
7b2b9d6 
Renamed local variable in sstable::total_reclaimable_memory_size in
preparation for the next patch which adds a new member variable
_total_memory_reclaimed to the sstable class.

Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit a53af1f)
@lkshminarayanan @mergify
sstable: track memory reclaimed from components per sstable
fdb983f 
Added a member variable _total_memory_reclaimed to the sstable class
that tracks the total memory reclaimed from a sstable.

Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit 02d272f)
@lkshminarayanan @mergify
sstable: renamed intrusive list link type
2b5bfbb 
Renamed the intrusive list link type to differentiate it from the set
link type that will be added in an upcoming patch.

Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit 3ef2f79)
@lkshminarayanan @mergify
sstable: add link and comparator class to support new instrusive set
b5b9b9d 
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit 140d887)

# Conflicts:
#	sstables/sstables.hh
@lkshminarayanan @mergify
sstables_manager: add new intrusive set to track the reclaimed sstables
a164754 
The new set holds the sstables from where the memory has been reclaimed
and is sorted in ascending order of the total memory reclaimed.

Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit 2340ab6)
@lkshminarayanan @mergify
sstables: support reloading reclaimed components
37272b7 
Added support to reload components from which memory was previously
reclaimed as the total memory of reclaimable components crossed a
threshold. The implementation is kept simple as only the bloom filters
are considered reclaimable for now.

Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit 54bb03c)
@lkshminarayanan @mergify
sstable_datafile_test: add test to verify reclaimed components reload
02e052c 
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit 69b2a12)
@lkshminarayanan @mergify
sstable_directory_test: fix generation in sstable_directory_test_tabl…
5c9afb0 
…e_scan_incomplete_sstables

The testcase uses an sstable whose mutation key and the generation are
owned by different shards. Due to this, when process_sstable_dir is
called, the sstable gets loaded into a different shard than the one that
was intended. This also means that the sstable and the sstable manager
end up in different shards.

The following patch will introduce a condition variable in sstables
manager which will be signalled from the sstables. If the sstable and
the sstable manager are in different shards, the signalling will cause
the testcase to fail in debug mode with this error : "Promise task was
set on shard x but made ready on shard y". So, fix it by supplying
appropriate generation number owned by the same shard which owns the
mutation key as well.

Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit 2406406)
@lkshminarayanan @mergify
sstables_manager: start a fiber to reload components
d81854d 
Start a fiber that gets notified whenever an sstable gets deleted. The
fiber doesn't do anything yet but the following patch will add support
to reload reclaimed components if there is sufficient memory.

Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit f758d7b)
@lkshminarayanan @mergify
sstables_manager: reload previously reclaimed components when memory …
35ae909 
…is available

When an SSTable is dropped, the associated bloom filter gets discarded
from memory, bringing down the total memory consumption of bloom
filters. Any bloom filter that was previously reclaimed from memory due
to the total usage crossing the threshold, can now be reloaded back into
memory if the total usage can still stay below the threshold. Added
support to reload such reclaimed filters back into memory when memory
becomes available.

Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit 0b06119)
@lkshminarayanan @mergify
sstable_datafile_test: add test to verify auto reload of reclaimed co…
b2a3746 
…mponents

Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit a080daa)
@lkshminarayanan @mergify
sstable_datafile_test: add testcase to test reclaim during reload
3f7d004 
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
(cherry picked from commit 4d22c4b)

# Conflicts:
#	sstables/sstables.cc
@mergify mergify bot requested a review from tgrabiec as a code owner May 11, 2024 21:45
@mergify mergify bot added the conflicts label May 11, 2024
@mergify mergify bot requested a review from nyh as a code owner May 11, 2024 21:45
@mergify Mergify
Copy link
Author

mergify bot commented May 11, 2024

Cherry-pick of 140d887 has failed:

On branch mergify/copy/branch-5.4/pr-18186
Your branch is ahead of 'origin/branch-5.4' by 3 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit 140d8871e1.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   sstables/sstables.hh

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of 4d22c4b has failed:

On branch mergify/copy/branch-5.4/pr-18186
Your branch is ahead of 'origin/branch-5.4' by 11 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit 4d22c4b68b.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   test/boost/sstable_datafile_test.cc
	modified:   test/lib/sstable_test_env.hh
	modified:   utils/error_injection.hh

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   sstables/sstables.cc

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot marked this pull request as draft May 11, 2024 21:46
@lkshminarayanan
Copy link
Member

Replaced by #18660

@mergify mergify bot deleted the mergify/copy/branch-5.4/pr-18186 branch May 15, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgrabiec tgrabiec Awaiting requested review from tgrabiec tgrabiec is a code owner

@nyh nyh Awaiting requested review from nyh nyh is a code owner

Assignees

@lkshminarayanan lkshminarayanan

Labels
conflicts
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@lkshminarayanan