-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Eventing framework refactor #6610
Eventing framework refactor #6610
Conversation
648e889
to
0a45e69
Compare
589b974
to
88755c3
Compare
334ae14
to
85b1460
Compare
03c047c
to
48446fe
Compare
48446fe
to
a1ba8da
Compare
a1ba8da
to
eefc466
Compare
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.
Thanks for doing all of this work. I left some notes in code that you may just be moving around. I think it's fine to leave it be if you want and we can follow up and fix in another diff.
a91d4c7
to
d212293
Compare
d212293
to
4616948
Compare
[[deprecated("Group events together and use addBatch() instead.")]] | ||
// clang-format on | ||
Status | ||
add(const Row& r); |
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.
nitpick, remove the newline within the function signature
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 tried to remove this newline but clang-format/make format will restore it, probably due to how the [[deprecated]] tag is handled
4616948
to
53281b3
Compare
This patch aims to move the event indexes outside the database to reduce the disk activity required to capture/evict/expire/return events.
A refactor has also taken place so that tests for this part of the code can be rewritten and improved to make sure regressions are not introduced.
Requires: #6732 (which is now done)