async icon indicating copy to clipboard operation
async copied to clipboard

A future-based drain for the asynchronous future

Open nagisa opened this issue 6 years ago • 7 comments

With futures and async coming into the rust standard library, proper, perhaps slog-async should grow some support for a Future to act as the other end of a Drain? This would allow one to spawn all of the logging on an executor which will inevitably exist in any Rust project of 2020.

This is what code would end up looking like:

let (drain, future) = slog_async::Something::new();
runtime.spawn(future); // don’t have to think about this again, and runs on the same runtime as everything else.
let logger = slog::Logger::root(drain, ...);

Implementation wise Drain will likely become the sender end of some async-friendly implementation of channel, whereas the future would be implemented in a way resembling this:

async {
    loop { 
        let message = async_channel.pop().await;
        some_fd.poll_write(..., message).await;
    }
}

Sadly, this likely cannot be done in a composable manner with other already existing drains, as those are synchronous.

nagisa avatar Oct 10 '19 03:10 nagisa

Hello

This is more of my own view than the projects view of the thing, but…

If I follow you correctly, you want to save that one thread that just submits the logs to a file or somewhere. While I myself don't like having too many threads around, I'm not sure this is worth the effort, at least right now.

This would allow one to spawn all of the logging on an executor which will inevitably exist in any Rust project of 2020.

I kind of doubt that. While async is a big hype now, not everyone needs that and not everyone needs an executor therefore.

Second, async logging is used not only to avoid the IO, but to offload the CPU overhead of formatting the messages.

But most importantly, as you said, this needs changes in other parts of the slog infrastructure. Currently, the Async is not in the place of the final drain. It feeds the messages to other drains and it only does the offloading to other thread. While you certainly could define some new structure that plays the role of the very final drain and runs on an executor, this is quite far from what slog-async does and could as well go to its own crate.

On the other hand, I'd certainly see some advantage in supporting the warn! and debug! macros to be async on top of any drain. But again, that's a whole project feature.

So, as this seems to be somewhat bigger problem, do you think it makes sense to discuss it in the context of the whole slog?

vorner avatar Oct 11 '19 11:10 vorner

So, as this seems to be somewhat bigger problem, do you think it makes sense to discuss it in the context of the whole slog?

Maybe, I filled it out here, as it seemed more appropriate here at first, and the rest I investigated while typing down the issue text.

nagisa avatar Oct 11 '19 12:10 nagisa

Since such drains would not compose with existing drains, they might as well be a part of a different ecosystem alog or aslog or something.

Well.. maybe we could add async fn log_async(...) with a default impl to a trait Drain...

But then there's a lot of other stuff to glue with it. There's a whole tracing crate for tokio IIRC. Something to consider.

And all in all, I would like to reiterate @vorner - I would still prefer to have a separate thread to offload the cpu load, and one additional thread for the whole application is not a problem. So I don't know how much value there is in such an endeavor.

dpc avatar Oct 11 '19 19:10 dpc

I don't think the point should be to avoid creating a thread. Instead, my concern is that in async code we should optimally not block. I think it would be appropriate to at least provide a futures module which provides the same functionality as in the blocking version, but using futures where it would otherwise block, for example when sending things on a channel.

Ploppz avatar Mar 06 '21 13:03 Ploppz

AFAIK, slog-async already doesn't block unless you tell it to https://docs.rs/slog-async/2.6.0/slog_async/enum.OverflowStrategy.html

dpc avatar Mar 10 '21 22:03 dpc

Oh, true. So slog is currently perfectly suitable for use in async applications.

Ploppz avatar Mar 13 '21 16:03 Ploppz

AFAIK, yes.

dpc avatar Mar 15 '21 06:03 dpc