feat: Add flush method on Drain trait

[gabyx]

Closes: slog-rs/async#35
This needs to be thoroughly reviewed, not sure if its good enough =)

• The return type io:Result<()> could not be turned into result::Result<Self::Ok, Self::Err> since that would need some more redirection or, or I use result::Result<Self::Ok, Self::Err> and return Ok(Self::Ok::new()) if possible in the default flush() implementation.

Make sure to:

• Add an entry to CHANGELOG.md (if necessary)

[Techcable]

I think this is an excellent addition! Relying on an explicit mem::drop doesn't always flush correctly in the presence of Logger::clone.

I've wanted this for a while. Thank you for adding it :)

[Techcable]

Right now, the thread:sleep in Async::flush seems surprising to me.

It's specific to the Async logger. If the user switches to another logger like slog-term, it would just flush the underlying .

I think there are two options to make the waiting more explicit. I would appreciate your input on them.

1. Return a FlushStatus

Instead of a function that returns io::Result<()>, it should return io::Result<FlushStatus>:

enum FlushStatus {
    Success,
    QueuedEntries,
}

Then the user can call thread:sleep if they really want to block until success.

2. Pass a explicit FlushWait

enum FlushWait {
    NoWait,
    BlockUntilEmpty(Duration)
}

Then it's more explicit that passing a duration would cause the loop to block. Let me know what you think about these ideas.

[gabyx]

@Techcable: Thanks for the review. Cool if its usable =). I was not yet sure about the function either. Your suggestion sounds very good. Both options are totally legit and better then what I provided!.

The only thing is can we make it such that the signature for an another flush e.g. on term also makes sense?
With both approaches you suggest there is still a slight mismatch, either QueuedEntries (term does not have that, and might just return Success anyway), and NoWait makes no sense for term but BlockUntilEmpty(duration) also not because duration is probably obsolete. But I like the type-guarded input! or the output.

Maybe the first one makes more sense, however thinking about what you use in generic code is a Logger where you dont know the underlying thing. So you also do not really care about QueuedEntries.
A flush() would be nicest, but does not work for Async, so we need somekind of duration if we block inside.
On the other hand if we dont block inside and let the user handle this, its kind of weird that you can call flush and then for some logger its not guaranteed that it happened and you need to run a loop.

Furthermore: What I do not quite understand so far ist:
The blocking loop which we do, does that really work?
since when you have a producing thread (which produces records fast) and a thread which calls flush at the same time, the loop might never return, because there is never a window where the queue is empty???
Doesn't the queue need something like a DoFlush message (?) which will increment a counting semaphore by 1 (or mutexed integer) which then gets handled by the worker which then decreases the counting semaphore by 1 (until 0), the flush awaits just till the counter reached the level it had before atomically incrementing it, only then we know that all message before that DoFlush message (timestamp wise) are flushed.

1. Queue: a,b,c,d,e
   Flush Counter: 0

2. Some thread 1 requests a flush
   Queue: Flush(0) a,b,c,d,e
   Flush Counter: 1

3. Some more message arrive, some have been handled
   Queue: k, j, l , Flush(0)
   Flush Counter: 1

4. Some thread 2 requests a flush
   Queue: Flush(1), k, j, l, Flush(0) a,b,c,d,e
   Flush Counter: 2

5. Flush(0) is handled == decrement the counter which notifies the thread.
   Queue: Flush(1), k, j, l
   Flush Counter: 1

6. Thread 1 continues because its flush has been handled.

Might that be a solution which makes more sense?

[Techcable]

Hi @gabyx ! Thank you for your contribution. I have decided to reject this PR in favor of the similar API in #349 .

The new Drain::flush method is present in the slog v2.8.0 release. It should be supported in slog_json and slog_term very soon, although slog_async might take a little longer for me to implement.