Use Bifrost Appender API in Shuffler #2659
Conversation
There was a problem hiding this comment.
Thanks for creating this PR @slinkydeveloper. I am not 100% sure whether caching the Appender makes a significant performance improvement (best if we measured it). It seems that creating an Appender involves a few atomic variable writes. What could make a bigger difference is to use the BackgroundAppender. But this requires a bigger refactoring. Hence, I would suggest to close this PR unless the measurements say something else.
| move |msg| { | ||
| let bifrost = bifrost.clone(); | ||
| async move { | ||
| append_envelope_to_bifrost(&bifrost, Arc::new(msg)).await?; |
There was a problem hiding this comment.
I am not sure whether creating Appenders is so expensive. It looks that we might save a few atomic writes. Did you measure the impact of your changes? What could make a difference is the usage of the BackgroundAppender which also supports batching of appends. But this will require a bigger refactoring of the shuffler to make it work, I believe.
No description provided.