Re: Add jsonlog log_destination for JSON server logs

On Thu, Sep 16, 2021 at 9:36 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> I am not really impressed by 0001 and the introduction of
> LOG_DESTINATIONS_WITH_FILES. So I would leave that out and keep the
> list of destinations listed instead. And we are talking about two
> places here, only within syslogger.c.
>

I've taken that out for now. The idea was to simplify the additions when
jsonlog is added but can circle back to that later if it makes sense.

> 0002 looks like an improvement,

Nice. That's left unchanged (renamed to 0001 now).

> but I think that 0003 makes the
> readability of the code worse with the introduction of eight static
> routines to handle all those cases.
>
> file_log_dest_should_rotate_for_size(), file_log_dest_close(),
> file_log_dest_check_rotate_for_size(), get_syslogger_file() don't
> bring major improvements. On the contrary,
> file_log_dest_write_metadata and file_log_dest_rotate seem to reduce a
> bit the noise.
>

It was helpful to split them out while working on the patch but I see your
point upon re-reading through the result.

Attached version (renamed to 002) adds only three static functions for
checking rotation size, performing the actual rotation, and closing. Also
one other to handle generating the logfile names with a suffix to handle
the pfree-ing (wrapping logfile_open(...)).

The rest of the changes happen in place using the new structures.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/