Ensure stats progress and tag individual stale stats #482
Conversation
Bound the time that stats calculation can take
|
should be reviewed with basho/riak_kv#763, see that PR for testing notes and strategies. |
| {error, Reason} -> | ||
| lager:debug("stat calc failed: ~p ~p", [Reason]), | ||
| {Reply, State1} = | ||
| case OldValue of |
There was a problem hiding this comment.
I prefer tag_stale/1 had a clause that matched <<"stale:", _/binary>> and you could lose this case statement, and keep the knowledge about tagging encapsulated in the tag_stale/1 function, or is the state mutation too expensive in that case since you'd be setting State#state{value=V} even if it is unchanged.
|
I like it. I'm curious about your aggressive short line length policy, though. Sometimes I think it makes the code less readable. |
| @@ -140,6 +174,12 @@ do_calc_stat(Stat) -> | |||
| spawn_link( | |||
| fun() -> | |||
| StatVal = riak_core_stat_q:calc_stat(Stat), | |||
| gen_server:cast(ServerPid, {value, StatVal, folsom_utils:now_epoch()}) end | |||
| gen_server:cast(ServerPid, {value, StatVal, | |||
There was a problem hiding this comment.
this one because I thought that it looked better with end easier to match with fun, and the rest because it was long line, which are the most horrible thing in the entire universe.
|
Oh, I meant to say +1 |
|
I suppose that sometimes the long-line thing might make things harder to read. I think that in erlang it's particularly bad, because of how the default modes push things to the right quite quickly, especially in anonymous functions, which are handled more like right-hand-rule stuff than just adding another level of scope indention. Anyway, I can revert, I don't really care, it's just that 80 columns is a hard habit to break. |
|
updated, if you want to take another look. |
|
All the ones are plussed. Merge at will. Many thanks. |
Ensure stats progress and tag individual stale stats
2.0 version of #467, with additional tagging of individual stale stats, even though the rest of the stat cache and the overall cache timestamps may continue to progress.
cc @rsltrifork @russelldb