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
mon/LogMonitor: separate out summary by channel #21395
Conversation
@jecluis I did the lame thing since it is quick and easy. This should avoid the major pain point for mimic while we sort out a proper refactor. |
Instead of sending the last 1 message, send from the last committed segment. This may be >1 entry, which is a bit odd, but otherwise the behavior is similar. Note that since the channel filtering is happening on the client this may mean a higher chance of getting something visible at all. Signed-off-by: Sage Weil <sage@redhat.com>
87164fd
to
5d99708
Compare
Instead of keeping the last N entries, keep the last N entries for each channel. This ensures that lots of audit records don't age out the cluster records (or vice versa). The overall approach does not change, and that approach is overall pretty lame. We're still rewriting a big summary blob on every log commit. We still should refactor this later. This solves the immediate problem at a small cost of increasing the log summary structure by ~2x. Signed-off-by: Sage Weil <sage@redhat.com>
5d99708
to
648aaf2
Compare
summary.build_ordered_tail(&full_tail); | ||
derr << "full " << full_tail << dendl; | ||
auto rp = full_tail.rbegin(); | ||
for (; num > 0 && rp != full_tail.rend(); ++rp) { |
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.
Can we avoid writing out this loop twice? Maybe pull it up into a lambda at the start of the function and then apply it to full_tail
or tail_by_channel.find(channel)
conditionally
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.
It's awkward because one is a list of entries and the other is a list of pairs.. not sure how to generalize that?
mlog->put(); | ||
return; | ||
} | ||
_create_sub_incremental(mlog, sub_level, get_last_committed()); |
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.
Is this going to work if the latest message at sub_level
is in an older revision than get_last_committed
?
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.
Never mind, the old _create_sub_summary was only looking at the current summary anyway
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.
LGTM, just a comment on the repetition in preprocess_command
This is a stopgap to keep N entires per channel instead of N total, preventing one log channel
(e.g., audit) from aging out entries from antoher channel (e.g., cluster).
We should still do a larger restructure/refactor on LogMonitor later.