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

feat(server): tighten memory checks when inseting a new object. #258

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

romange
Copy link
Collaborator

@romange romange commented Aug 25, 2022

Before this change Dragonfly evicted items only when it was low on memory and had to grow its main dictionary.
It is not enough because in many cases Dragonfly can grow in memory even when the main dictionary does not grow.
For example, when its dictionary is less than 90% utilized but the newly added objects require lots of memory.

In addition, the dashtable could add additional segments when others had enough available slots to fill the rest of the free memory.

This change adds another layer of defense that allows evicting items even when dictionary segments are not full.
The eviction is still local with respect to a segment. On my tests it seems that it's much harder to cross maxmemory limit than before.

In addition we improved our heuristic that allowed the dashtable to grow. Now it takes into account the average bytes per item in order to
project how much memory the full table takes before adding to it new segments. This really tightens dashtable utilization.

Things to improve:

  1. the eviction behavior is rough. Once an insert does the eviction it tries to free enough objects to bring memory back.
    This may result in very spiky insertion/eviction patterns.
  2. The eviction procedure, even though it's limited to a single segment, is quite heavy because it goes over all buckets
    in the segment. This may result in weird artifacts where we evict just enough to be under the limit, then add and evict
    again and so on.
  3. Therefore, we may need a periodic eviction that will complement this emergency eviction step.

Fixes #224 and partially addresses #256

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

Before this change Dragonfly evicted items only when it was low on memory and had to grow its main dictionary.
It is not enough because in many cases Dragonfly can grow in memory even when the main dictionary does not grow.
For example, when its dictionary is less than 90% utilized, but the newly added objects require lots of memory.

In addition, the dashtable adds additional segments, when other segments have enough available slots to fill the rest of the free memory.

This change adds another layer of defense that allows evicting items even when dictionary segments are not full.
The eviction is still local with respect to a segment. On my tests it seems that it's much harder to cross maxmemory limit than before.

In addition, we tightened the heuristic that allowes the dashtable to grow. Now it takes into account the average bytes per item
in order to project how much memory the full table takes before adding to it new segments.
This really improves dashtable utilization.

There are still things to improve:
1. the eviction behavior is rough. Once an insert does the eviction it tries to free enough objects to bring memory back.
   This may result in very spiky insertion/eviction patterns.
2. The eviction procedure, even though it's limited to a single segment, is quite heavy because it goes over all buckets
   in the segment. This may result in weird artifacts where we evict just enough to be under the limit, then add and evict
   again and so on.
3. Therefore, we may need a periodic eviction that will compliment this emergency eviction step.

Fixes #224 and partially addresses #256

Signed-off-by: Roman Gershman <[email protected]>
@romange romange merged commit 4e4ed63 into main Aug 26, 2022
@romange romange deleted the HardenMemBound branch August 26, 2022 12: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.

Dragonfly does not respect maxmemory limit
1 participant