-
Notifications
You must be signed in to change notification settings - Fork 118
[Cleaner] remove the cleaner external bifrost writer #3987
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
Conversation
7c63aa8 to
f21eef3
Compare
1ec113e to
cec1475
Compare
6e4eac3 to
13c995f
Compare
tillrohrmann
left a comment
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 removing the external bifrost writer from the cleaner task @muhamadazmy. The changes look good to me. Did you see any negative impact on the partition processor loop by these changes? If not, then +1 for merging :-)
|
@tillrohrmann I ran few tests with very aggressive cleanup timer (and very short retention periods) and I did not observe any regression in performance compared to main. |
c41ecb6 to
11e5095
Compare
af82da1 to
8b37c97
Compare
tillrohrmann
left a comment
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. +1 for merging :-)
faf9958 to
eaccc70
Compare
bb244b9 to
384853c
Compare
Summary: Switch to bilrost encoding
Summary: This PR makes sure cleaner does not do an external bifrost write by using creating a cleaner effect stream that can be handled directly by the PP event loop
[Cleaner] remove the cleaner external bifrost writer
Summary:
This PR makes sure cleaner does not do an external bifrost write
by using creating a cleaner effect stream that can be handled
directly by the PP event loop
Stack created with Sapling. Best reviewed with ReviewStack.
Shuffler#4024IngestionClientfor invocation and state mgmt #3980ingestion-client#3975