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

fix(server): fix the bug where DF went over maxmemory limit #683

Merged
merged 1 commit into from
Jan 16, 2023

Conversation

romange
Copy link
Collaborator

@romange romange commented Jan 16, 2023

Reproducible scenario: zipfantest -c 20000000 -d 2000 -a 0.95 -p 10 -upper_bound 2000000 ,
when dragonfly runs with: --maxmemory=1G --cache_mode --proactor_threads=2

Problem:

  1. Inside CanGrow() check we did not account for the additional segment when doing the test against the memory budget.
  2. Our soft budget limit was too low, preventing more detailed checks to stop the allocation.

Signed-off-by: Roman Gershman [email protected]

@romange romange force-pushed the TuneEvictionPolicy branch from 88e27ef to 232a661 Compare January 16, 2023 06:00
@romange romange requested a review from dranikpg January 16, 2023 06:01
@dranikpg
Copy link
Contributor

It seems like this is conflicting with your tiered storage

…ntly

Reproducible scenario: `zipfantest -c 20000000 -d 2000 -a 0.95 -p 10 -upper_bound 2000000` when
dragonfly runs with: `--maxmemory=1G --cache_mode --proactor_threads=2`

1. Inside CanGrow() check we did not account for the additional segment when doing the test against
   the memory budget.
2. Our soft budget limit was too low, preventing more detailed checks to stop the allocation.

Signed-off-by: Roman Gershman <[email protected]>
@romange romange force-pushed the TuneEvictionPolicy branch from 232a661 to 671677f Compare January 16, 2023 09:11
@romange
Copy link
Collaborator Author

romange commented Jan 16, 2023

Not conflicting. It exposed the flakiness. I rebased now

@romange romange merged commit 9610038 into main Jan 16, 2023
@romange romange deleted the TuneEvictionPolicy branch January 16, 2023 10:00
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