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

scale allocator member ItemLimit to the relative share of ItemSlots, and refine suspension eligibility #416

Merged
merged 2 commits into from
Jan 21, 2025

Conversation

jgraettinger
Copy link
Contributor

@jgraettinger jgraettinger commented Jan 17, 2025

ItemSlots may be unevenly distributed amoung assignment sub-problems.

We need to account for this, by scaling member ItemLimits by the relative portion of assignment slots that must be allocated within each sub-problem, rather than just the relative number of items.

Also, tune journal suspension to apply the journal's configured flush interval when evaluating whether the index is "dirty".

See individual commits.


This change is Reviewable

@jgraettinger jgraettinger requested a review from psFried January 17, 2025 20:04
Item replication may not be evenly distributed.

Previously, we were not accounting for this within each allocation
sub-problem, and scaled member ItemLimits by the relative ratio of
items in each sub-problem rather than the relative ratio of item *slots*.

This caused excessive re-assignement of items in solved maximum
assignment flows, as push/relabel must spend more time back-tracking as
it overflows the initial constraints on item limits.

Introduce a new BenchmarkChangingReplication which reproduces this
effect.

Then, solve it by tweaking sparse flow network construction to scale
a member ItemLimit by the relative portion of assignment slots that
must be allocated within each sub-problem (rather than just the
relative number of items).

BenchmarkChangingReplication confirms this heuristic update dramatically
reduces the degree of assignment churn (note `run.ratio`, which is a
measure of average churn per item replication change):

Before:
    final metrics    adds=142806 packs=90617 removes=83044 run.ratio=16.58809060161935 run.total=9757
After:
    final metrics    adds=69854 packs=4457 removes=10092 run.ratio=1.6343138259710976 run.total=9757
@jgraettinger jgraettinger force-pushed the johnny/release-pipeline branch from 3fe9bed to f510dce Compare January 18, 2025 19:53
@jgraettinger jgraettinger changed the title allocator: scale member ItemLimit to the relative share of ItemSlots scale allocator member ItemLimit to the relative share of ItemSlots, and refine suspension eligibility Jan 18, 2025
Previously, a journal was eliglble for suspension as soon as all of its
fragments were persisted.

Though this is safe and correct, it doesn't account for raced evaluations
in the broader context of a busy cluster with other topology changes and
journal churn. Specifically, a journal with regular appends may be
temporarily eligble for suspension only because its toplogy was recently
changed, causing its fragments to be flushed.

Worse, the journal's suspension (and soon, re-activation) may trigger churn
which causes *other* journals to flush and themselves become suspended.

Groups of journals may also have correlated write schedules, such as journals
which are written once daily at midnight. We'd like to avoid thundering
herds of journal suspensions and re-activations that might impact other
journals due to assignment churn.

To resolve this, prevent a journal from being suspended until its
fragments are fully remote AND are at-least as old as its configured
flush interval (or 24h, if not defined).
@jgraettinger jgraettinger force-pushed the johnny/release-pipeline branch from 5277ddd to d700583 Compare January 18, 2025 23:04
Copy link
Contributor

@psFried psFried left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jgraettinger jgraettinger merged commit f172f5b into master Jan 21, 2025
1 check passed
@jgraettinger jgraettinger deleted the johnny/release-pipeline branch January 21, 2025 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants