Link phosphor-hostlogger and bmcweb

[Nan Zhou]

>
> >
> > > we propose to remove the ring buffer and write to the log file
> > > as soon as some characters are received.
> >
> > I am not sure it is a good idea.
> > The host can generate a lot of logs, but we have very limited free space.
> This is a fair concern, but wouldn't the rollover limits make this not
> an issue?  They seem like they could be easily configured.

Right. Logrotate will be able to handle the rotation. Maximum size, # log
files, and compression can be easily configured.

> In
> > addition, such heavy traffic will lead to a quick breakdown of the flash
> (most
> > available products are guaranteed to withstand around 100,000 P/E cycles
> only).
> JFFS2 is wear leveled, and there are other BMC stacks that I know of
> that implement this without any ill effects to flash longevity, with
> that said, if Nan made the "last log on disk" feature configurable,
> would that alleviate your concerns?

We also noticed that the obmc-server itself will buffer the log a bit. Will
it still be a problem if we don't write a character at once but a block of
them?
And as Ed said, we can also make this feature configurable. I would imagine
the log buffer will remain if the "last log on disk" feature is disabled.


> >
> > > This implicitly removes the needs
> > > of the ring buffer, and the persistence triggering (host reboot,
> sigterm,
> > > etc) in hostlogger. We believe this keeps the same functionality but
> saves
> > > hundreds of lines of codes in phosphor-hostlogger.
> Difference of opinion here, I don't think this removes the need for
> the host reboot event;  Having each reboot post to a different log
> needs to be maintained, and I have to imagine that there's some sort
> of sigterm handler still, although it becomes a lot smaller.


>
> > You are suggesting to delete the buffer, DBus watcher, log rotate. How
> are you
> > going to keep the same functionality if you remove everything related to
> it?
> +1.  In the initial thought I didn't think we were removing any
> functionality with this.  I had assumed the dbus watcher would remain,
> and we would still have the log rotation behavior.  In reading through
> Nans proposal I don't think these are getting removed;  Maybe I
> misunderstood?


Yes, if we want to keep different reboot posts to a different log file, we
can keep part of the dbus/signal watcher.

On Fri, May 21, 2021 at 10:24 AM Ed Tanous <edtanous at google.com> wrote:

> On Thu, May 20, 2021 at 11:10 PM Artem Senichev <artemsen at gmail.com>
> wrote:
> >
> > On Thu, May 20, 2021 at 04:29:09PM -0700, Nan Zhou wrote:
> > > Hi all,
> > >
> > > In the previous thread (
> > > https://lists.ozlabs.org/pipermail/openbmc/2021-March/025234.html), we
> > > (engineers from Google and Quanta) discussed our attempt to share host
> > > serial logs via Redfish, which includes polling logs via LogService and
> > > streaming log bytes via EventService (e.g. SSE). We went with the
> event log
> > > architecture
> > > <
> https://github.com/openbmc/docs/blob/master/architecture/redfish-logging-in-bmcweb.md
> >
> > > and did the implementation.
> > >
> > > We still want to reuse the phosphor-hostlogger and do some
> modification. We
> > > believe it is better to try to reuse existing codes if possible and
> improve
> > > them rather than creating new things that have similar functionalities
> (in
> > > our case, it is a daemon that could collect logs and persist them).
> >
> > I agree, reusing code is a right choice, but only when it is really
> possible.
> > For now it looks like you want to remove most of the Hostlogger features
> to
> > transform it from buffer-like to stream-like service.
>
> I'm not quite following this statement.  Which features are getting
> removed?  From what I can see, he's suggesting making
> phosphor-hostlogger look more like a well-behaved linux logging
> daemon, but I don't think any features got omitted (or I'm missing
> something critical).
>
> >
> > > We want to do the following modification in phosphor-hostlogger (from
> the
> > > input and output point of view, just very few things will be changed)
> > >
> > > 1. One limitation of phosphor-hostlogger is that it will lose data in
> the
> > > memory (the ring buffer maintained by hostlogger) when BMC gets force
> > > restarted;
> >
> > When BMC goes to reboot it stops all services, at that moment hostlogger
> gets
> > a signal and flushes all gathered logs to a file.
>
> Only if the reboot is planned.  If the BMC loses power (which is
> "normal" for a bmc) there isn't time to persist to flash before the
> power goes down and the logs are most likely lost.
>
> >
> > > we propose to remove the ring buffer and write to the log file
> > > as soon as some characters are received.
> >
> > I am not sure it is a good idea.
> > The host can generate a lot of logs, but we have very limited free space.
>
> This is a fair concern, but wouldn't the rollover limits make this not
> an issue?  They seem like they could be easily configured.
>
> > In
> > addition, such heavy traffic will lead to a quick breakdown of the flash
> (most
> > available products are guaranteed to withstand around 100,000 P/E cycles
> only).
>
> JFFS2 is wear leveled, and there are other BMC stacks that I know of
> that implement this without any ill effects to flash longevity, with
> that said, if Nan made the "last log on disk" feature configurable,
> would that alleviate your concerns?
>
> >
> > > This implicitly removes the needs
> > > of the ring buffer, and the persistence triggering (host reboot,
> sigterm,
> > > etc) in hostlogger. We believe this keeps the same functionality but
> saves
> > > hundreds of lines of codes in phosphor-hostlogger.
>
> Difference of opinion here, I don't think this removes the need for
> the host reboot event;  Having each reboot post to a different log
> needs to be maintained, and I have to imagine that there's some sort
> of sigterm handler still, although it becomes a lot smaller.
>
> >
> > You are suggesting to delete the buffer, DBus watcher, log rotate. How
> are you
> > going to keep the same functionality if you remove everything related to
> it?
>
> +1.  In the initial thought I didn't think we were removing any
> functionality with this.  I had assumed the dbus watcher would remain,
> and we would still have the log rotation behavior.  In reading through
> Nans proposal I don't think these are getting removed;  Maybe I
> misunderstood?
>
> >
> > > 2. We propose not to compress the latest log file. This saves us the
> > > overhead of doing decompression when BMCWeb just needs to retrieve the
> most
> > > recent logs. There are still going to be log rotations in the file
> level.
> > > Files other than the latest log file are still going to be compressed.
> We
> > > can modify existing codes to achieve this or use the linux logrotate
> > > directly.
> > >
> > > Furthermore, we will add host serial logs into BMCWeb, both LogService
> and
> > > EventService. In LogService, we will teach BMCWeb how to read the
> latest
> > > log file that is not compressed and the other compressed old logs, and
> how
> > > to assemble LogEntries out of raw serial logs. We will discuss
> EventService
> > > in future threads but the very initial idea is to setup inotify on log
> > > files and SSE to stream out new bytes to clients (like what the
> existing
> > > event logging is doing).
> > >
> > > As we said above, for phosphor-hostlogger, the input is still the
> > > obmc-server unix socket, and the output are still persisted log files.
> But
> > > the functionality will get improved (less data loss), code gets
> simplified
> > > (no ring buffer and persistence triggering), and it will become easier
> and
> > > more performant to get BMCWeb hooked up for log streaming via Redfish.
> > >
> > > Please let us know what you think. We appreciate any feedback. Thank
> you
> > > very much!
> > >
> > > Sincerely,
> > > Nan
> >
> > --
> > Regards,
> > Artem Senichev
> > Software Engineer, YADRO.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20210521/6e3b5ee0/attachment.htm>