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

Exometer cached value potentially not deleted when expired #105

Open
lucafavatella opened this issue Jan 4, 2018 · 1 comment
Open

Exometer cached value potentially not deleted when expired #105

lucafavatella opened this issue Jan 4, 2018 · 1 comment

Comments

@lucafavatella
Copy link

When a process calls exometer_cache:write/{3,4}, it:

  • Asynchronously (cast) instructs the exometer_cache process to start a timer for the (metric-datapoint keyed) cache deletion, and to update (ets:update_element) the relevant field in the ETS entry with the timer ref;
  • It replaces (ets:insert) the ETS entry, without timer ref.

Depending on the scheduling of the process calling exometer_cache:write and the exometer_cache process (I am excluding process death for simplicity), the cache ETS entry may or may not have the timer ref.
If the cache ETS entry has no timer ref, the cache ETS entry is not deleted when the timer fires.

A following exometer_cache:write may update the cache ETS entry, though it looks to me this is not the intended behaviour hence the report.


Sample minimal example (notice the ets:select_delete calls):

Erlang/OTP 20 [erts-9.2] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:10] [hipe] [kernel-poll:false]

Eshell V9.2  (abort with ^G)
1> ets:new(?TABLE, [set, public, named_table, {keypos, 2}]).
* 1: syntax error before: '?'
1> ets:new(exometer_cache, [set, public, named_table, {keypos, 2}]).
exometer_cache
2> ets:insert(exometer_cache, {cache, name, value, undefined, time, ttl}).
true
3> ets:lookup(exometer_cache, name).
[{cache,name,value,undefined,time,ttl}]
4> ets:update_element(exometer_cache, name, {4, tref}).
true
5> ets:lookup(exometer_cache, name).
[{cache,name,value,tref,time,ttl}]
6> ets:update_element(exometer_cache, name, {4, tref2}).
true
7> ets:lookup(exometer_cache, name).
[{cache,name,value,tref2,time,ttl}]
8> ets:insert(exometer_cache, {cache, name, value, undefined, time, ttl}).
true
9> ets:lookup(exometer_cache, name).
[{cache,name,value,undefined,time,ttl}]
10> ets:select_delete(exometer_cache, [{{cache, name, '_', tref2, '_', '_'}, [], [true]}]).
0
11> ets:lookup(exometer_cache, name).
[{cache,name,value,undefined,time,ttl}]
12> ets:update_element(exometer_cache, name, {4, tref3}).
true
13> ets:select_delete(exometer_cache, [{{cache, name, '_', tref3, '_', '_'}, [], [true]}]).
1
14> ets:lookup(exometer_cache, name).
[]
@lucafavatella
Copy link
Author

A following exometer_cache:write may update the cache ETS entry, though it looks to me this is not the intended behaviour hence the report.

Actually I see that the piece of code that I understand it is supposed to call exometer_cache:write on a metric-datapoint meant to be cached is conditionally called based on the metric-datapoint having a cached value, so this seems an issue.

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

No branches or pull requests

1 participant