-
Notifications
You must be signed in to change notification settings - Fork 455
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
Make the high water mark shaper better #411
Conversation
* If the shaper is in overload and the final message comes in, but no further messages arrive for some time, until another message came in, the drop count would not be printed. Now we set a timer to ensure it prints the drop count after the current second expires. * Allow the shaper to take a filter function that allows events that would not normally be printed anyway to not be counted against the HWM. This means that if you're suppressing supervisor startup messages you won't see drop events counted for messages you'd never see printed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this is awesome work. Had a few requests to use named functions instead of anonymous ones.
src/error_logger_lager_h.erl
Outdated
fun(_) -> false end; | ||
{true, true} -> | ||
fun({info_report, _GL, {_Pid, std_info, D}}) when is_list(D) -> | ||
lists:member({exited, stopped}, D); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would really like to see this anonymous fun in its own named function - I think it will make the code clearer and any stacktraces more obvious.
src/error_logger_lager_h.erl
Outdated
false | ||
end; | ||
{false, true} -> | ||
fun({info_report, _GL, {_Pid, std_info, D}}) when is_list(D) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above. Would you please make this its own named function?
src/error_logger_lager_h.erl
Outdated
false | ||
end; | ||
{true, false} -> | ||
fun({info_report, _GL, {_P, progress, D}}) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
Reworked as requested. |
👍 Thanks! I think this is great! |
further messages arrive for some time, until another message came in,
the drop count would not be printed. Now we set a timer to ensure it
prints the drop count after the current second expires.
would not normally be printed anyway to not be counted against the
HWM. This means that if you're suppressing supervisor startup messages
you won't see drop events counted for messages you'd never see
printed.