Introduce hard span storage size limit in sweeper #121
Conversation
deadtrickster
force-pushed
the
size-sweeper
branch
from
9c27fca
to
e82f879
Compare
Codecov Report
@@ Coverage Diff @@
## master #121 +/- ##
==========================================
- Coverage 80.32% 79.86% -0.46%
==========================================
Files 35 35
Lines 737 750 +13
==========================================
+ Hits 592 599 +7
- Misses 145 151 +6
Continue to review full report at Codecov.
|
| ttl=TTL, | ||
| storage_size=MaxSize}) -> | ||
|
|
||
| StorageSize = ets:info(?SPAN_TAB, memory) * erlang:system_info({wordsize, external}), |
There was a problem hiding this comment.
Shouldn't bother with these when storage_size is infinity.
|
I'm not opposed to this, but I do think something like this makes more sense to effect the sampling. Like a sampler that lowers the probability relative to the size of the table. |
|
well, 'overflow' could happen probably with any sampler other than none. So to me it's a bit different thing, like literally what shields from crashes. Looking forward to more fancy samples though |
|
Sure, and the sampler that lowers based on size could go all the way to |
|
but it means that only this particular sampler cares about table size, not all of them? I mean if this code stays here all samplers are covered, otherwise I have to stick to one sampler |
src/oc_span_sweeper.erl
Outdated
| ttl=TTL}) when is_function(Fun) -> | ||
| Expired = select_expired(TTL), | ||
| [Fun(Span) || Span <- Expired], | ||
| {keep_state_and_data, [{next_event, internal, just_do_it}, |
There was a problem hiding this comment.
What is this new internal event?
There was a problem hiding this comment.
copy-pasta, at the beginning I wanted to first sweep by ttl and then memory, that's a left over
src/oc_span_sweeper.erl
Outdated
| storage_size=MaxSize}) -> | ||
|
|
||
| StorageSize = case MaxSize of | ||
| infinity -> |
There was a problem hiding this comment.
Not a big deal, but personally I'd find it cleaner to be like:
do_gc(#data{strategy=Strategy,
ttl=TTL,
storage_size=infinity}) ->
sweep_spans(Strategy, TTL);
do_gc(#data{strategy=Strategy,
ttl=TTL,
storage_size=MaxSize}) ->
StorageSize = ets:info(?SPAN_TAB, memory) * erlang:system_info({wordsize, external}),
...There was a problem hiding this comment.
was thinking about this, thought would be ugly, but now I see it's not
|
Elvis complains about |
|
Weird, yea, I would ignore that from elvis. |
|
Looks like you need to fix up the dialyzer ignores as well and then this'll be good to go. |
deadtrickster
force-pushed
the
size-sweeper
branch
from
08282e6
to
efb2a87
Compare
Let's discuss while I'm thinking about tests