Skip to content
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

A future-based drain for the asynchronous future #13

Open
nagisa opened this issue Oct 10, 2019 · 7 comments
Open

A future-based drain for the asynchronous future #13

nagisa opened this issue Oct 10, 2019 · 7 comments

Comments

@nagisa
Copy link

nagisa commented Oct 10, 2019

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.

@vorner
Copy link
Collaborator

vorner commented Oct 11, 2019

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?

@nagisa
Copy link
Author

nagisa commented Oct 11, 2019

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.

@dpc
Copy link
Contributor

dpc commented Oct 11, 2019

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.

@Ploppz
Copy link

Ploppz commented Mar 6, 2021

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.

@dpc
Copy link
Contributor

dpc commented Mar 10, 2021

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

@Ploppz
Copy link

Ploppz commented Mar 13, 2021

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

@dpc
Copy link
Contributor

dpc commented Mar 15, 2021

AFAIK, yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants