search

pinojs / sonic-boom

Extremely fast utf8 only stream implementation
MIT License
266 stars 41 forks source link

Add an option for periodic flush #193

Closed segevfiner closed 2 months ago

segevfiner commented 8 months ago

When used for logging there could be cases of quiet periods where some log messages are buffered, but otherwise not yet flushed as they haven't filled the buffer yet. It would be helpful if sonic-boom would flush such messages after enough time passed without any new messages getting written so as you can still see what's the latest thing that has been logged in such cases without having to turn on sync mode.

segevfiner commented 8 months ago

Actually... Why is anything getting buffered in the first place when I use this via pino-pretty without any minLength... Something is buffering messages sync or not...

segevfiner commented 8 months ago

Probably due to this https://github.com/pinojs/pino/issues/1801

segevfiner commented 8 months ago

Anyhow I think that this is only relevant with minLength set, AKA actual buffering taking place, which it isn't by default as far as I can see. So I'm probably experiencing something starving the event loop.

mcollina commented 8 months ago

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

forgotPassword commented 7 months ago

+1. Using set/clearInterval and calling .flush() seems to work for now.

mcollina commented 7 months ago

I think that's easy enough and it does not required any special handling to prevent leaking. Would you like to send a PR to document it in the docs?

forgotPassword commented 7 months ago

Flush will throw an error if sb.destroyed, so you either have to try,catch this call or pass a do_nothing callback, which in turn sets and removes several drain events each time, seems wasteful for this case. As this library has several subtleties it would be great if someone who is more familiar with the code base added this functionality natively.