All of lore.kernel.org
 help / color / mirror / Atom feed
* Link phosphor-hostlogger and bmcweb
@ 2021-05-20 23:29 Nan Zhou
  2021-05-21  6:10 ` Artem Senichev
  0 siblings, 1 reply; 23+ messages in thread
From: Nan Zhou @ 2021-05-20 23:29 UTC (permalink / raw)
  To: openbmc
  Cc: Spencer Ku, Litzung Chen, a.amelkin, Ed Tanous, Richard Hanley,
	a.senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 3116 bytes --]

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.

However, thanks to the maintainers of host-logger, we found the limitation
that we might flood the systemd-journal. Given that serial logs are special
and new data comes more often than almost anything else on the BMC,
flooding systemd-journal will be a fatal problem.

So we are thinking about other directions. The current idea is that we
don't involve DBus at all and BMCWeb reads serial logs from log files
directly.

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).

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; we propose to remove the ring buffer and write to the log file
as soon as some characters are received. 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.

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

[-- Attachment #2: Type: text/html, Size: 3549 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-20 23:29 Link phosphor-hostlogger and bmcweb Nan Zhou
@ 2021-05-21  6:10 ` Artem Senichev
  2021-05-21 15:10   ` Nan Zhou
  2021-05-21 17:23   ` Ed Tanous
  0 siblings, 2 replies; 23+ messages in thread
From: Artem Senichev @ 2021-05-21  6:10 UTC (permalink / raw)
  To: Nan Zhou
  Cc: Spencer Ku, Litzung Chen, openbmc, a.amelkin, Ed Tanous,
	Richard Hanley, a.senichev, a.filippov

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.

> 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.

> 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. 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).

> 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.

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?

> 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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-21  6:10 ` Artem Senichev
@ 2021-05-21 15:10   ` Nan Zhou
  2021-05-21 17:25     ` Ed Tanous
  2021-05-21 17:23   ` Ed Tanous
  1 sibling, 1 reply; 23+ messages in thread
From: Nan Zhou @ 2021-05-21 15:10 UTC (permalink / raw)
  To: Artem Senichev
  Cc: Spencer Ku, Litzung Chen, openbmc, a.amelkin, Ed Tanous,
	Richard Hanley, a.senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 7143 bytes --]

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.

Hostlogger will still consume bytes from obmc-console-server and persist
logs in files. If the log itself has EOL, it will be splitted in the file
as well.
I thought that is all things we want in the Hostlogger.

> 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.

Yes, I understand hostlogger registers a sigterm handler. But what if BMC
lost its power before sigterm gets sent? That's what I mean by "force
restart".

> 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.
> 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).

I would like to get input from Ed for this point. +Ed Tanous
<edtanous@google.com>.

> 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.
> 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?

Log rotation is kept. Ring buffer and DBus watcher are great in the
original design but become unnecessary if we persist logs immediately. Do
we miss
anything that the previous hostlogger provides (transfer bytes from the
unix socket to EOL-separated compressed logs)? I believe in this proposal
we have
less data loss, simpler codes, and make it more performant for Redfish
integration.

On Thu, May 20, 2021 at 11:10 PM Artem Senichev <artemsen@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.
>
> > 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.
>
> > 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.
> 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).
>
> > 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.
>
> 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?
>
> > 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.
>

[-- Attachment #2: Type: text/html, Size: 10804 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-21  6:10 ` Artem Senichev
  2021-05-21 15:10   ` Nan Zhou
@ 2021-05-21 17:23   ` Ed Tanous
  2021-05-21 17:51     ` Nan Zhou
  1 sibling, 1 reply; 23+ messages in thread
From: Ed Tanous @ 2021-05-21 17:23 UTC (permalink / raw)
  To: Artem Senichev
  Cc: Spencer Ku, Nan Zhou, Litzung Chen, OpenBMC Maillist,
	Alexander Amelkin, Richard Hanley, a.senichev, a.filippov

On Thu, May 20, 2021 at 11:10 PM Artem Senichev <artemsen@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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-21 15:10   ` Nan Zhou
@ 2021-05-21 17:25     ` Ed Tanous
  0 siblings, 0 replies; 23+ messages in thread
From: Ed Tanous @ 2021-05-21 17:25 UTC (permalink / raw)
  To: Nan Zhou
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	a.senichev, Richard Hanley, Artem Senichev, a.filippov

On Fri, May 21, 2021 at 8:11 AM Nan Zhou <nanzhou@google.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.
>
> Hostlogger will still consume bytes from obmc-console-server and persist logs in files. If the log itself has EOL, it will be splitted in the file as well.
> I thought that is all things we want in the Hostlogger.
>
>> > 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.
>
> Yes, I understand hostlogger registers a sigterm handler. But what if BMC lost its power before sigterm gets sent? That's what I mean by "force restart".
>
> > 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. 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).
>
> I would like to get input from Ed for this point. +Ed Tanous.

Gave my input on the previous email before I read this one.  Lets
follow this up on that thread.

>
>> > 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.
>> 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?
>
> Log rotation is kept. Ring buffer and DBus watcher are great in the original design but become unnecessary if we persist logs immediately. Do we miss
> anything that the previous hostlogger provides (transfer bytes from the unix socket to EOL-separated compressed logs)? I believe in this proposal we have
> less data loss, simpler codes, and make it more performant for Redfish integration.
>
> On Thu, May 20, 2021 at 11:10 PM Artem Senichev <artemsen@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.
>>
>> > 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.
>>
>> > 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. 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).
>>
>> > 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.
>>
>> 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?
>>
>> > 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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-21 17:23   ` Ed Tanous
@ 2021-05-21 17:51     ` Nan Zhou
  2021-05-24  7:52       ` Artem Senichev
  0 siblings, 1 reply; 23+ messages in thread
From: Nan Zhou @ 2021-05-21 17:51 UTC (permalink / raw)
  To: Ed Tanous
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	a.senichev, Richard Hanley, Artem Senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 8010 bytes --]

>
> >
> > > 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@google.com> wrote:

> On Thu, May 20, 2021 at 11:10 PM Artem Senichev <artemsen@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.
>

[-- Attachment #2: Type: text/html, Size: 11322 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-21 17:51     ` Nan Zhou
@ 2021-05-24  7:52       ` Artem Senichev
  2021-05-24 16:27         ` Ed Tanous
  0 siblings, 1 reply; 23+ messages in thread
From: Artem Senichev @ 2021-05-24  7:52 UTC (permalink / raw)
  To: Nan Zhou
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

I'll try to convey the main idea that we tried to implement in this service.

Hostlogger was originally designed to work with OpenPOWER systems, which
generate a very detailed log at boot time.
It is important to save these logs and the host console output just before
rebooting for further investigation of incidents when hardware errors occur.
So, we have two log files for each server session (boot log + last OS messages).
That's why we need a D-bus watcher.
BMC flash has around 3MiB of free RW space, this force us to use compression
and file rotation.

All of these features are unnecessary for "streaming" real-time log recording.
You don't need DBus watchers, rotation can be done with native Linux utilities,
you don't even need to split the input stream into lines.
Just redirect obmc-console.log: `tail -f /var/log/obmc-console.log > my.log`.

I understand your desire to add a new mode to the existing project instead of
creating a new one. But there is very little in common between these modes.
Even reading the socket will have to be done separately, since it is buffered
for line splitting in the current implementation.
In the end, only bb-recipe and the `main` function will remain in the common. 

-- 
Regards,
Artem Senichev
Software Engineer, YADRO.

On Fri, May 21, 2021 at 10:51:45AM -0700, Nan Zhou wrote:

> >
> > >
> > > > 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@google.com> wrote:
> 
> > On Thu, May 20, 2021 at 11:10 PM Artem Senichev <artemsen@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.
> >

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-24  7:52       ` Artem Senichev
@ 2021-05-24 16:27         ` Ed Tanous
  2021-05-25  6:41           ` Artem Senichev
  0 siblings, 1 reply; 23+ messages in thread
From: Ed Tanous @ 2021-05-24 16:27 UTC (permalink / raw)
  To: Artem Senichev
  Cc: Spencer Ku, Nan Zhou, Litzung Chen, OpenBMC Maillist,
	Alexander Amelkin, Richard Hanley, a.senichev, a.filippov

On Mon, May 24, 2021 at 12:52 AM Artem Senichev <artemsen@gmail.com> wrote:
>
> I'll try to convey the main idea that we tried to implement in this service.
>
> Hostlogger was originally designed to work with OpenPOWER systems, which
> generate a very detailed log at boot time.

There are definitely other non OpenPOWER systems that have this same behavior.

> It is important to save these logs and the host console output just before
> rebooting for further investigation of incidents when hardware errors occur.
> So, we have two log files for each server session (boot log + last OS messages).
> That's why we need a D-bus watcher.
> BMC flash has around 3MiB of free RW space, this force us to use compression
> and file rotation.
>
> All of these features are unnecessary for "streaming" real-time log recording.

I disagree with you there.  Rotation and compression are still useful
in a "streaming" case.  Because of the ways the APIs are defined,
LogService in redfish provides both a "historical" version of past
logs.  It's useful to have those logs rotated and compressed.

> You don't need DBus watchers, rotation can be done with native Linux utilities,
> you don't even need to split the input stream into lines.

I'm not following why those now wouldn't be needed.  Redfish LogEntry
would separate per line, so we'd have to do the splitting somewhere.
There's already code to do that in hostlogger.  Wouldn't you still
want to separate log per boot, and have lines split between log files?
 I'm not following why those would go away just because there's a
desire to poll for logs and get up to date information.

> Just redirect obmc-console.log: `tail -f /var/log/obmc-console.log > my.log`.

This doesn't solve the problem presented.  First of all, log rotation
and compression are still needed.  Also, it's desirable to have dbus
watchers and separate the logs per boot, such that they can end up
separated in the Redfish API.  Also, in what you presented, my.log
would quickly and easily overflow the available space, as there's no
log rotation.

>
> I understand your desire to add a new mode to the existing project instead of
> creating a new one. But there is very little in common between these modes.

I'm not following how they're all that different, see above about
needing very similar features.  For the sake of argument, lets say we
went with a totally different implementation, would it be able to live
in the hostlogger repo to be able to reuse code where needed?  There's
a lot of code that I suspect will be identical.

> Even reading the socket will have to be done separately, since it is buffered
> for line splitting in the current implementation.
> In the end, only bb-recipe and the `main` function will remain in the common.
>
> --
> Regards,
> Artem Senichev
> Software Engineer, YADRO.
>
> On Fri, May 21, 2021 at 10:51:45AM -0700, Nan Zhou wrote:
>
> > >
> > > >
> > > > > 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@google.com> wrote:
> >
> > > On Thu, May 20, 2021 at 11:10 PM Artem Senichev <artemsen@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.
> > >

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-24 16:27         ` Ed Tanous
@ 2021-05-25  6:41           ` Artem Senichev
  2021-05-25 15:03             ` Nan Zhou
  0 siblings, 1 reply; 23+ messages in thread
From: Artem Senichev @ 2021-05-25  6:41 UTC (permalink / raw)
  To: Ed Tanous
  Cc: Spencer Ku, Nan Zhou, Litzung Chen, OpenBMC Maillist,
	Alexander Amelkin, Richard Hanley, a.senichev, a.filippov

Sorry guys, maybe this is a misunderstanding on my part.

I was confused with the following line in the proposal:

"we propose to remove the ring buffer and write to the log file as soon
as some characters are received. This implicitly removes the needs of
the ring buffer, and the persistence triggering (host reboot, sigterm,
etc) in hostlogger"

I would like to get a more detailed description of further changes in order
to see the whole picture of the solution. 

--
Regards,
Artem Senichev
Software Engineer, YADRO.


On Mon, May 24, 2021 at 09:27:39AM -0700, Ed Tanous wrote:
> On Mon, May 24, 2021 at 12:52 AM Artem Senichev <artemsen@gmail.com> wrote:
> >
> > I'll try to convey the main idea that we tried to implement in this service.
> >
> > Hostlogger was originally designed to work with OpenPOWER systems, which
> > generate a very detailed log at boot time.
> 
> There are definitely other non OpenPOWER systems that have this same behavior.
> 
> > It is important to save these logs and the host console output just before
> > rebooting for further investigation of incidents when hardware errors occur.
> > So, we have two log files for each server session (boot log + last OS messages).
> > That's why we need a D-bus watcher.
> > BMC flash has around 3MiB of free RW space, this force us to use compression
> > and file rotation.
> >
> > All of these features are unnecessary for "streaming" real-time log recording.
> 
> I disagree with you there.  Rotation and compression are still useful
> in a "streaming" case.  Because of the ways the APIs are defined,
> LogService in redfish provides both a "historical" version of past
> logs.  It's useful to have those logs rotated and compressed.
> 
> > You don't need DBus watchers, rotation can be done with native Linux utilities,
> > you don't even need to split the input stream into lines.
> 
> I'm not following why those now wouldn't be needed.  Redfish LogEntry
> would separate per line, so we'd have to do the splitting somewhere.
> There's already code to do that in hostlogger.  Wouldn't you still
> want to separate log per boot, and have lines split between log files?
>  I'm not following why those would go away just because there's a
> desire to poll for logs and get up to date information.
> 
> > Just redirect obmc-console.log: `tail -f /var/log/obmc-console.log > my.log`.
> 
> This doesn't solve the problem presented.  First of all, log rotation
> and compression are still needed.  Also, it's desirable to have dbus
> watchers and separate the logs per boot, such that they can end up
> separated in the Redfish API.  Also, in what you presented, my.log
> would quickly and easily overflow the available space, as there's no
> log rotation.
> 
> >
> > I understand your desire to add a new mode to the existing project instead of
> > creating a new one. But there is very little in common between these modes.
> 
> I'm not following how they're all that different, see above about
> needing very similar features.  For the sake of argument, lets say we
> went with a totally different implementation, would it be able to live
> in the hostlogger repo to be able to reuse code where needed?  There's
> a lot of code that I suspect will be identical.
> 
> > Even reading the socket will have to be done separately, since it is buffered
> > for line splitting in the current implementation.
> > In the end, only bb-recipe and the `main` function will remain in the common.
> >
> > --
> > Regards,
> > Artem Senichev
> > Software Engineer, YADRO.
> >
> > On Fri, May 21, 2021 at 10:51:45AM -0700, Nan Zhou wrote:
> >
> > > >
> > > > >
> > > > > > 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@google.com> wrote:
> > >
> > > > On Thu, May 20, 2021 at 11:10 PM Artem Senichev <artemsen@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.
> > > >

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-25  6:41           ` Artem Senichev
@ 2021-05-25 15:03             ` Nan Zhou
  2021-05-26  5:29               ` Nan Zhou
  0 siblings, 1 reply; 23+ messages in thread
From: Nan Zhou @ 2021-05-25 15:03 UTC (permalink / raw)
  To: Artem Senichev
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 14281 bytes --]

>
> "we propose to remove the ring buffer and write to the log file as soon
> as some characters are received. This implicitly removes the needs of
> the ring buffer, and the persistence triggering (host reboot, sigterm,
> etc) in hostlogger"
> I would like to get a more detailed description of further changes in order
> to see the whole picture of the solution.

 Originally I didn't consider including changing the log file according to
the boot cycle; we will keep part of the dbus/signal watcher to make that
different reboot posts to a different log file.

We will work out some more detailed descriptions for future changes soon.

On Mon, May 24, 2021 at 11:41 PM Artem Senichev <artemsen@gmail.com> wrote:

> Sorry guys, maybe this is a misunderstanding on my part.
>
> I was confused with the following line in the proposal:
>
> "we propose to remove the ring buffer and write to the log file as soon
> as some characters are received. This implicitly removes the needs of
> the ring buffer, and the persistence triggering (host reboot, sigterm,
> etc) in hostlogger"
>
> I would like to get a more detailed description of further changes in order
> to see the whole picture of the solution.
>
> --
> Regards,
> Artem Senichev
> Software Engineer, YADRO.
>
>
> On Mon, May 24, 2021 at 09:27:39AM -0700, Ed Tanous wrote:
> > On Mon, May 24, 2021 at 12:52 AM Artem Senichev <artemsen@gmail.com>
> wrote:
> > >
> > > I'll try to convey the main idea that we tried to implement in this
> service.
> > >
> > > Hostlogger was originally designed to work with OpenPOWER systems,
> which
> > > generate a very detailed log at boot time.
> >
> > There are definitely other non OpenPOWER systems that have this same
> behavior.
> >
> > > It is important to save these logs and the host console output just
> before
> > > rebooting for further investigation of incidents when hardware errors
> occur.
> > > So, we have two log files for each server session (boot log + last OS
> messages).
> > > That's why we need a D-bus watcher.
> > > BMC flash has around 3MiB of free RW space, this force us to use
> compression
> > > and file rotation.
> > >
> > > All of these features are unnecessary for "streaming" real-time log
> recording.
> >
> > I disagree with you there.  Rotation and compression are still useful
> > in a "streaming" case.  Because of the ways the APIs are defined,
> > LogService in redfish provides both a "historical" version of past
> > logs.  It's useful to have those logs rotated and compressed.
> >
> > > You don't need DBus watchers, rotation can be done with native Linux
> utilities,
> > > you don't even need to split the input stream into lines.
> >
> > I'm not following why those now wouldn't be needed.  Redfish LogEntry
> > would separate per line, so we'd have to do the splitting somewhere.
> > There's already code to do that in hostlogger.  Wouldn't you still
> > want to separate log per boot, and have lines split between log files?
> >  I'm not following why those would go away just because there's a
> > desire to poll for logs and get up to date information.
> >
> > > Just redirect obmc-console.log: `tail -f /var/log/obmc-console.log >
> my.log`.
> >
> > This doesn't solve the problem presented.  First of all, log rotation
> > and compression are still needed.  Also, it's desirable to have dbus
> > watchers and separate the logs per boot, such that they can end up
> > separated in the Redfish API.  Also, in what you presented, my.log
> > would quickly and easily overflow the available space, as there's no
> > log rotation.
> >
> > >
> > > I understand your desire to add a new mode to the existing project
> instead of
> > > creating a new one. But there is very little in common between these
> modes.
> >
> > I'm not following how they're all that different, see above about
> > needing very similar features.  For the sake of argument, lets say we
> > went with a totally different implementation, would it be able to live
> > in the hostlogger repo to be able to reuse code where needed?  There's
> > a lot of code that I suspect will be identical.
> >
> > > Even reading the socket will have to be done separately, since it is
> buffered
> > > for line splitting in the current implementation.
> > > In the end, only bb-recipe and the `main` function will remain in the
> common.
> > >
> > > --
> > > Regards,
> > > Artem Senichev
> > > Software Engineer, YADRO.
> > >
> > > On Fri, May 21, 2021 at 10:51:45AM -0700, Nan Zhou wrote:
> > >
> > > > >
> > > > > >
> > > > > > > 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@google.com>
> wrote:
> > > >
> > > > > On Thu, May 20, 2021 at 11:10 PM Artem Senichev <
> artemsen@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.
> > > > >
>

[-- Attachment #2: Type: text/html, Size: 19542 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-25 15:03             ` Nan Zhou
@ 2021-05-26  5:29               ` Nan Zhou
  2021-05-26  6:11                 ` Artem Senichev
  0 siblings, 1 reply; 23+ messages in thread
From: Nan Zhou @ 2021-05-26  5:29 UTC (permalink / raw)
  To: Artem Senichev
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 16239 bytes --]

Hi Artem,

I listed the potential changes below (to the best of my understand of
phosphor-hostlogger),

1. log_buffer.xpp:
will be removed; we propose to persist a chunk into the log file as soon as
we receive it; newline will be logged as is, so logs are still splitted by
newlines; we can potentially set the maximum length of a log as well (force
split a long line into smaller lines)

2. host_console.xpp: stay unchanged;

3. zlib_file.xpp, zlib_exception.xpp:
will be removed or slightly changed; we can potentially use the linux
logrotate which has built-in compression and file rotation (in this case
these compression utilities will be removed).
The latest log file isn't compressed any more. History log files are still
compressed.
We need to implement some codes to deal with the race condition in log
rotations (we should reopen the writing fd after the latest log file is
renamed; inotify might do the trick).
We might also need to rotate according to host boot cycles (like what
postcodes are doing right now).

3. dbus_loop.xpp: stay unchanged;

4. service.xpp:
will be slightly changed; socket IO callback and host state watcher are
kept; manual flash or flash upon service restart will be removed

5. config.xpp:
will be slightly changed; options for flash policy will be removed;

We might split the implementation into several phases. We might not jump to
the final version at one iteration. But the above are what we eventually
want at this moment.

On Tue, May 25, 2021 at 8:03 AM Nan Zhou <nanzhou@google.com> wrote:

> "we propose to remove the ring buffer and write to the log file as soon
>> as some characters are received. This implicitly removes the needs of
>> the ring buffer, and the persistence triggering (host reboot, sigterm,
>> etc) in hostlogger"
>> I would like to get a more detailed description of further changes in
>> order
>> to see the whole picture of the solution.
>
>  Originally I didn't consider including changing the log file according to
> the boot cycle; we will keep part of the dbus/signal watcher to make that
> different reboot posts to a different log file.
>
> We will work out some more detailed descriptions for future changes soon.
>
> On Mon, May 24, 2021 at 11:41 PM Artem Senichev <artemsen@gmail.com>
> wrote:
>
>> Sorry guys, maybe this is a misunderstanding on my part.
>>
>> I was confused with the following line in the proposal:
>>
>> "we propose to remove the ring buffer and write to the log file as soon
>> as some characters are received. This implicitly removes the needs of
>> the ring buffer, and the persistence triggering (host reboot, sigterm,
>> etc) in hostlogger"
>>
>> I would like to get a more detailed description of further changes in
>> order
>> to see the whole picture of the solution.
>>
>> --
>> Regards,
>> Artem Senichev
>> Software Engineer, YADRO.
>>
>>
>> On Mon, May 24, 2021 at 09:27:39AM -0700, Ed Tanous wrote:
>> > On Mon, May 24, 2021 at 12:52 AM Artem Senichev <artemsen@gmail.com>
>> wrote:
>> > >
>> > > I'll try to convey the main idea that we tried to implement in this
>> service.
>> > >
>> > > Hostlogger was originally designed to work with OpenPOWER systems,
>> which
>> > > generate a very detailed log at boot time.
>> >
>> > There are definitely other non OpenPOWER systems that have this same
>> behavior.
>> >
>> > > It is important to save these logs and the host console output just
>> before
>> > > rebooting for further investigation of incidents when hardware errors
>> occur.
>> > > So, we have two log files for each server session (boot log + last OS
>> messages).
>> > > That's why we need a D-bus watcher.
>> > > BMC flash has around 3MiB of free RW space, this force us to use
>> compression
>> > > and file rotation.
>> > >
>> > > All of these features are unnecessary for "streaming" real-time log
>> recording.
>> >
>> > I disagree with you there.  Rotation and compression are still useful
>> > in a "streaming" case.  Because of the ways the APIs are defined,
>> > LogService in redfish provides both a "historical" version of past
>> > logs.  It's useful to have those logs rotated and compressed.
>> >
>> > > You don't need DBus watchers, rotation can be done with native Linux
>> utilities,
>> > > you don't even need to split the input stream into lines.
>> >
>> > I'm not following why those now wouldn't be needed.  Redfish LogEntry
>> > would separate per line, so we'd have to do the splitting somewhere.
>> > There's already code to do that in hostlogger.  Wouldn't you still
>> > want to separate log per boot, and have lines split between log files?
>> >  I'm not following why those would go away just because there's a
>> > desire to poll for logs and get up to date information.
>> >
>> > > Just redirect obmc-console.log: `tail -f /var/log/obmc-console.log >
>> my.log`.
>> >
>> > This doesn't solve the problem presented.  First of all, log rotation
>> > and compression are still needed.  Also, it's desirable to have dbus
>> > watchers and separate the logs per boot, such that they can end up
>> > separated in the Redfish API.  Also, in what you presented, my.log
>> > would quickly and easily overflow the available space, as there's no
>> > log rotation.
>> >
>> > >
>> > > I understand your desire to add a new mode to the existing project
>> instead of
>> > > creating a new one. But there is very little in common between these
>> modes.
>> >
>> > I'm not following how they're all that different, see above about
>> > needing very similar features.  For the sake of argument, lets say we
>> > went with a totally different implementation, would it be able to live
>> > in the hostlogger repo to be able to reuse code where needed?  There's
>> > a lot of code that I suspect will be identical.
>> >
>> > > Even reading the socket will have to be done separately, since it is
>> buffered
>> > > for line splitting in the current implementation.
>> > > In the end, only bb-recipe and the `main` function will remain in the
>> common.
>> > >
>> > > --
>> > > Regards,
>> > > Artem Senichev
>> > > Software Engineer, YADRO.
>> > >
>> > > On Fri, May 21, 2021 at 10:51:45AM -0700, Nan Zhou wrote:
>> > >
>> > > > >
>> > > > > >
>> > > > > > > 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@google.com>
>> wrote:
>> > > >
>> > > > > On Thu, May 20, 2021 at 11:10 PM Artem Senichev <
>> artemsen@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.
>> > > > >
>>
>

[-- Attachment #2: Type: text/html, Size: 21597 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-26  5:29               ` Nan Zhou
@ 2021-05-26  6:11                 ` Artem Senichev
  2021-05-26  6:51                   ` Nan Zhou
  0 siblings, 1 reply; 23+ messages in thread
From: Artem Senichev @ 2021-05-26  6:11 UTC (permalink / raw)
  To: Nan Zhou
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

On Tue, May 25, 2021 at 10:29:48PM -0700, Nan Zhou wrote:
> Hi Artem,
> 
> I listed the potential changes below (to the best of my understand of
> phosphor-hostlogger),
> 
> 1. log_buffer.xpp:
> will be removed; we propose to persist a chunk into the log file as soon as
> we receive it; newline will be logged as is, so logs are still splitted by
> newlines;

If you remove the newline handler we will lose the timestamps added for each
line.

> we can potentially set the maximum length of a log as well (force
> split a long line into smaller lines)

I would leave the host output "as is". I don't think we should modify it.

> 2. host_console.xpp: stay unchanged;
> 
> 3. zlib_file.xpp, zlib_exception.xpp:
> will be removed or slightly changed; we can potentially use the linux
> logrotate which has built-in compression and file rotation (in this case
> these compression utilities will be removed).
> The latest log file isn't compressed any more. History log files are still
> compressed.

Just curious, how are you going to remove the oldest messages from the latest
file in runtime? You are not going to rewrite the entire file on every input
character, are you?

> We need to implement some codes to deal with the race condition in log
> rotations (we should reopen the writing fd after the latest log file is
> renamed; inotify might do the trick).
> We might also need to rotate according to host boot cycles (like what
> postcodes are doing right now).
>
> 3. dbus_loop.xpp: stay unchanged;
> 
> 4. service.xpp:
> will be slightly changed; socket IO callback and host state watcher are
> kept; manual flash or flash upon service restart will be removed

The manual flush is used in the dreport, please don't forget to remove that
call there.

> 5. config.xpp:
> will be slightly changed; options for flash policy will be removed;

Not slightly =)
Most options are related to the buffer and flush policies.
As I can see, there will stay the socket Id and output directory only.

> We might split the implementation into several phases. We might not jump to
> the final version at one iteration. But the above are what we eventually
> want at this moment.
> 
> On Tue, May 25, 2021 at 8:03 AM Nan Zhou <nanzhou@google.com> wrote:
> 
> > "we propose to remove the ring buffer and write to the log file as soon
> >> as some characters are received. This implicitly removes the needs of
> >> the ring buffer, and the persistence triggering (host reboot, sigterm,
> >> etc) in hostlogger"
> >> I would like to get a more detailed description of further changes in
> >> order
> >> to see the whole picture of the solution.
> >
> >  Originally I didn't consider including changing the log file according to
> > the boot cycle; we will keep part of the dbus/signal watcher to make that
> > different reboot posts to a different log file.
> >
> > We will work out some more detailed descriptions for future changes soon.
> >
> > On Mon, May 24, 2021 at 11:41 PM Artem Senichev <artemsen@gmail.com>
> > wrote:
> >
> >> Sorry guys, maybe this is a misunderstanding on my part.
> >>
> >> I was confused with the following line in the proposal:
> >>
> >> "we propose to remove the ring buffer and write to the log file as soon
> >> as some characters are received. This implicitly removes the needs of
> >> the ring buffer, and the persistence triggering (host reboot, sigterm,
> >> etc) in hostlogger"
> >>
> >> I would like to get a more detailed description of further changes in
> >> order
> >> to see the whole picture of the solution.
> >>
> >> --
> >> Regards,
> >> Artem Senichev
> >> Software Engineer, YADRO.
> >>
> >>
> >> On Mon, May 24, 2021 at 09:27:39AM -0700, Ed Tanous wrote:
> >> > On Mon, May 24, 2021 at 12:52 AM Artem Senichev <artemsen@gmail.com>
> >> wrote:
> >> > >
> >> > > I'll try to convey the main idea that we tried to implement in this
> >> service.
> >> > >
> >> > > Hostlogger was originally designed to work with OpenPOWER systems,
> >> which
> >> > > generate a very detailed log at boot time.
> >> >
> >> > There are definitely other non OpenPOWER systems that have this same
> >> behavior.
> >> >
> >> > > It is important to save these logs and the host console output just
> >> before
> >> > > rebooting for further investigation of incidents when hardware errors
> >> occur.
> >> > > So, we have two log files for each server session (boot log + last OS
> >> messages).
> >> > > That's why we need a D-bus watcher.
> >> > > BMC flash has around 3MiB of free RW space, this force us to use
> >> compression
> >> > > and file rotation.
> >> > >
> >> > > All of these features are unnecessary for "streaming" real-time log
> >> recording.
> >> >
> >> > I disagree with you there.  Rotation and compression are still useful
> >> > in a "streaming" case.  Because of the ways the APIs are defined,
> >> > LogService in redfish provides both a "historical" version of past
> >> > logs.  It's useful to have those logs rotated and compressed.
> >> >
> >> > > You don't need DBus watchers, rotation can be done with native Linux
> >> utilities,
> >> > > you don't even need to split the input stream into lines.
> >> >
> >> > I'm not following why those now wouldn't be needed.  Redfish LogEntry
> >> > would separate per line, so we'd have to do the splitting somewhere.
> >> > There's already code to do that in hostlogger.  Wouldn't you still
> >> > want to separate log per boot, and have lines split between log files?
> >> >  I'm not following why those would go away just because there's a
> >> > desire to poll for logs and get up to date information.
> >> >
> >> > > Just redirect obmc-console.log: `tail -f /var/log/obmc-console.log >
> >> my.log`.
> >> >
> >> > This doesn't solve the problem presented.  First of all, log rotation
> >> > and compression are still needed.  Also, it's desirable to have dbus
> >> > watchers and separate the logs per boot, such that they can end up
> >> > separated in the Redfish API.  Also, in what you presented, my.log
> >> > would quickly and easily overflow the available space, as there's no
> >> > log rotation.
> >> >
> >> > >
> >> > > I understand your desire to add a new mode to the existing project
> >> instead of
> >> > > creating a new one. But there is very little in common between these
> >> modes.
> >> >
> >> > I'm not following how they're all that different, see above about
> >> > needing very similar features.  For the sake of argument, lets say we
> >> > went with a totally different implementation, would it be able to live
> >> > in the hostlogger repo to be able to reuse code where needed?  There's
> >> > a lot of code that I suspect will be identical.
> >> >
> >> > > Even reading the socket will have to be done separately, since it is
> >> buffered
> >> > > for line splitting in the current implementation.
> >> > > In the end, only bb-recipe and the `main` function will remain in the
> >> common.
> >> > >
> >> > > --
> >> > > Regards,
> >> > > Artem Senichev
> >> > > Software Engineer, YADRO.
> >> > >
> >> > > On Fri, May 21, 2021 at 10:51:45AM -0700, Nan Zhou wrote:
> >> > >
> >> > > > >
> >> > > > > >
> >> > > > > > > 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@google.com>
> >> wrote:
> >> > > >
> >> > > > > On Thu, May 20, 2021 at 11:10 PM Artem Senichev <
> >> artemsen@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.
> >> > > > >
> >>
> >

-- 
Regards,
Artem Senichev
Software Engineer, YADRO.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-26  6:11                 ` Artem Senichev
@ 2021-05-26  6:51                   ` Nan Zhou
  2021-05-26  8:56                     ` Artem Senichev
  0 siblings, 1 reply; 23+ messages in thread
From: Nan Zhou @ 2021-05-26  6:51 UTC (permalink / raw)
  To: Artem Senichev
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 21974 bytes --]

> > 1. log_buffer.xpp:
> > will be removed; we propose to persist a chunk into the log file as soon
> as
> > we receive it; newline will be logged as is, so logs are still splitted
> by
> > newlines;
> If you remove the newline handler we will lose the timestamps added for
> each
> line.

Good catch. If timestamps are necessary (it is necessary for the Redfish
LogEntry), I think we can keep the current newline detection logic in
log_buffer.xpp. Once a new line is detected, we append the current
timestamp first, and then append the bytes chunk.

> we can potentially set the maximum length of a log as well (force
> > split a long line into smaller lines)
> I would leave the host output "as is". I don't think we should modify it.

OK. We have concerns about edge cases where a log line is too long.

> 2. host_console.xpp: stay unchanged;
> >
> > 3. zlib_file.xpp, zlib_exception.xpp:
> > will be removed or slightly changed; we can potentially use the linux
> > logrotate which has built-in compression and file rotation (in this case
> > these compression utilities will be removed).
> > The latest log file isn't compressed any more. History log files are
> still
> > compressed.
> Just curious, how are you going to remove the oldest messages from the
> latest
> file in runtime? You are not going to rewrite the entire file on every
> input
> character, are you?

The following is my current idea: we will rename the latest file to
something else and notify the writer (hostlogger) to close its old file
descriptor and open a new one (should be doable via linux logrotate and
inotify or some signal handlers, as logrotate is able to send some signals
to hostlogger if a rotation is performed). The writer keeps appending logs
most of the time using the same fd unless the latest file is rotated. This
should be better than truncating the file where the reader (BMCWeb) won't
have race conditions (it might read old snapshots but it is not a big deal
in our case).

> We need to implement some codes to deal with the race condition in log
> > rotations (we should reopen the writing fd after the latest log file is
> > renamed; inotify might do the trick).
> > We might also need to rotate according to host boot cycles (like what
> > postcodes are doing right now).
> >
> > 3. dbus_loop.xpp: stay unchanged;
> >
> > 4. service.xpp:
> > will be slightly changed; socket IO callback and host state watcher are
> > kept; manual flash or flash upon service restart will be removed
> The manual flush is used in the dreport, please don't forget to remove that
> call there.

Thanks for the information.

 > 5. config.xpp:
> > will be slightly changed; options for flash policy will be removed;
> Not slightly =)
> Most options are related to the buffer and flush policies.
> As I can see, there will stay the socket Id and output directory only.

Yes. I would assume the host state policy will stay as well.

How do you like the proposal now? I hope we make it more clear. Do you
think it is a good direction that we can go with?

On Tue, May 25, 2021 at 11:11 PM Artem Senichev <artemsen@gmail.com> wrote:

> On Tue, May 25, 2021 at 10:29:48PM -0700, Nan Zhou wrote:
> > Hi Artem,
> >
> > I listed the potential changes below (to the best of my understand of
> > phosphor-hostlogger),
> >
> > 1. log_buffer.xpp:
> > will be removed; we propose to persist a chunk into the log file as soon
> as
> > we receive it; newline will be logged as is, so logs are still splitted
> by
> > newlines;
>
> If you remove the newline handler we will lose the timestamps added for
> each
> line.
>
> > we can potentially set the maximum length of a log as well (force
> > split a long line into smaller lines)
>
> I would leave the host output "as is". I don't think we should modify it.
>
> > 2. host_console.xpp: stay unchanged;
> >
> > 3. zlib_file.xpp, zlib_exception.xpp:
> > will be removed or slightly changed; we can potentially use the linux
> > logrotate which has built-in compression and file rotation (in this case
> > these compression utilities will be removed).
> > The latest log file isn't compressed any more. History log files are
> still
> > compressed.
>
> Just curious, how are you going to remove the oldest messages from the
> latest
> file in runtime? You are not going to rewrite the entire file on every
> input
> character, are you?
>
> > We need to implement some codes to deal with the race condition in log
> > rotations (we should reopen the writing fd after the latest log file is
> > renamed; inotify might do the trick).
> > We might also need to rotate according to host boot cycles (like what
> > postcodes are doing right now).
> >
> > 3. dbus_loop.xpp: stay unchanged;
> >
> > 4. service.xpp:
> > will be slightly changed; socket IO callback and host state watcher are
> > kept; manual flash or flash upon service restart will be removed
>
> The manual flush is used in the dreport, please don't forget to remove that
> call there.
>
> > 5. config.xpp:
> > will be slightly changed; options for flash policy will be removed;
>
> Not slightly =)
> Most options are related to the buffer and flush policies.
> As I can see, there will stay the socket Id and output directory only.
>
> > We might split the implementation into several phases. We might not jump
> to
> > the final version at one iteration. But the above are what we eventually
> > want at this moment.
> >
> > On Tue, May 25, 2021 at 8:03 AM Nan Zhou <nanzhou@google.com> wrote:
> >
> > > "we propose to remove the ring buffer and write to the log file as soon
> > >> as some characters are received. This implicitly removes the needs of
> > >> the ring buffer, and the persistence triggering (host reboot, sigterm,
> > >> etc) in hostlogger"
> > >> I would like to get a more detailed description of further changes in
> > >> order
> > >> to see the whole picture of the solution.
> > >
> > >  Originally I didn't consider including changing the log file
> according to
> > > the boot cycle; we will keep part of the dbus/signal watcher to make
> that
> > > different reboot posts to a different log file.
> > >
> > > We will work out some more detailed descriptions for future changes
> soon.
> > >
> > > On Mon, May 24, 2021 at 11:41 PM Artem Senichev <artemsen@gmail.com>
> > > wrote:
> > >
> > >> Sorry guys, maybe this is a misunderstanding on my part.
> > >>
> > >> I was confused with the following line in the proposal:
> > >>
> > >> "we propose to remove the ring buffer and write to the log file as
> soon
> > >> as some characters are received. This implicitly removes the needs of
> > >> the ring buffer, and the persistence triggering (host reboot, sigterm,
> > >> etc) in hostlogger"
> > >>
> > >> I would like to get a more detailed description of further changes in
> > >> order
> > >> to see the whole picture of the solution.
> > >>
> > >> --
> > >> Regards,
> > >> Artem Senichev
> > >> Software Engineer, YADRO.
> > >>
> > >>
> > >> On Mon, May 24, 2021 at 09:27:39AM -0700, Ed Tanous wrote:
> > >> > On Mon, May 24, 2021 at 12:52 AM Artem Senichev <artemsen@gmail.com
> >
> > >> wrote:
> > >> > >
> > >> > > I'll try to convey the main idea that we tried to implement in
> this
> > >> service.
> > >> > >
> > >> > > Hostlogger was originally designed to work with OpenPOWER systems,
> > >> which
> > >> > > generate a very detailed log at boot time.
> > >> >
> > >> > There are definitely other non OpenPOWER systems that have this same
> > >> behavior.
> > >> >
> > >> > > It is important to save these logs and the host console output
> just
> > >> before
> > >> > > rebooting for further investigation of incidents when hardware
> errors
> > >> occur.
> > >> > > So, we have two log files for each server session (boot log +
> last OS
> > >> messages).
> > >> > > That's why we need a D-bus watcher.
> > >> > > BMC flash has around 3MiB of free RW space, this force us to use
> > >> compression
> > >> > > and file rotation.
> > >> > >
> > >> > > All of these features are unnecessary for "streaming" real-time
> log
> > >> recording.
> > >> >
> > >> > I disagree with you there.  Rotation and compression are still
> useful
> > >> > in a "streaming" case.  Because of the ways the APIs are defined,
> > >> > LogService in redfish provides both a "historical" version of past
> > >> > logs.  It's useful to have those logs rotated and compressed.
> > >> >
> > >> > > You don't need DBus watchers, rotation can be done with native
> Linux
> > >> utilities,
> > >> > > you don't even need to split the input stream into lines.
> > >> >
> > >> > I'm not following why those now wouldn't be needed.  Redfish
> LogEntry
> > >> > would separate per line, so we'd have to do the splitting somewhere.
> > >> > There's already code to do that in hostlogger.  Wouldn't you still
> > >> > want to separate log per boot, and have lines split between log
> files?
> > >> >  I'm not following why those would go away just because there's a
> > >> > desire to poll for logs and get up to date information.
> > >> >
> > >> > > Just redirect obmc-console.log: `tail -f
> /var/log/obmc-console.log >
> > >> my.log`.
> > >> >
> > >> > This doesn't solve the problem presented.  First of all, log
> rotation
> > >> > and compression are still needed.  Also, it's desirable to have dbus
> > >> > watchers and separate the logs per boot, such that they can end up
> > >> > separated in the Redfish API.  Also, in what you presented, my.log
> > >> > would quickly and easily overflow the available space, as there's no
> > >> > log rotation.
> > >> >
> > >> > >
> > >> > > I understand your desire to add a new mode to the existing project
> > >> instead of
> > >> > > creating a new one. But there is very little in common between
> these
> > >> modes.
> > >> >
> > >> > I'm not following how they're all that different, see above about
> > >> > needing very similar features.  For the sake of argument, lets say
> we
> > >> > went with a totally different implementation, would it be able to
> live
> > >> > in the hostlogger repo to be able to reuse code where needed?
> There's
> > >> > a lot of code that I suspect will be identical.
> > >> >
> > >> > > Even reading the socket will have to be done separately, since it
> is
> > >> buffered
> > >> > > for line splitting in the current implementation.
> > >> > > In the end, only bb-recipe and the `main` function will remain in
> the
> > >> common.
> > >> > >
> > >> > > --
> > >> > > Regards,
> > >> > > Artem Senichev
> > >> > > Software Engineer, YADRO.
> > >> > >
> > >> > > On Fri, May 21, 2021 at 10:51:45AM -0700, Nan Zhou wrote:
> > >> > >
> > >> > > > >
> > >> > > > > >
> > >> > > > > > > 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@google.com
> >
> > >> wrote:
> > >> > > >
> > >> > > > > On Thu, May 20, 2021 at 11:10 PM Artem Senichev <
> > >> artemsen@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.
> > >> > > > >
> > >>
> > >
>
> --
> Regards,
> Artem Senichev
> Software Engineer, YADRO.
>

[-- Attachment #2: Type: text/html, Size: 33899 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-26  6:51                   ` Nan Zhou
@ 2021-05-26  8:56                     ` Artem Senichev
  2021-05-26 15:17                       ` Nan Zhou
  0 siblings, 1 reply; 23+ messages in thread
From: Artem Senichev @ 2021-05-26  8:56 UTC (permalink / raw)
  To: Nan Zhou
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

On Tue, May 25, 2021 at 11:51:44PM -0700, Nan Zhou wrote:
> > > 3. zlib_file.xpp, zlib_exception.xpp:
> > > will be removed or slightly changed; we can potentially use the linux
> > > logrotate which has built-in compression and file rotation (in this case
> > > these compression utilities will be removed).
> > > The latest log file isn't compressed any more. History log files are
> > > still compressed.
> > Just curious, how are you going to remove the oldest messages from the
> > latest file in runtime? You are not going to rewrite the entire file on
> > every input character, are you?
> 
> The following is my current idea: we will rename the latest file to
> something else and notify the writer (hostlogger) to close its old file
> descriptor and open a new one (should be doable via linux logrotate and
> inotify or some signal handlers, as logrotate is able to send some signals
> to hostlogger if a rotation is performed). The writer keeps appending logs
> most of the time using the same fd unless the latest file is rotated. This
> should be better than truncating the file where the reader (BMCWeb) won't
> have race conditions (it might read old snapshots but it is not a big deal
> in our case).

Currently we can keep the last N lines of the host's output, the oldest
messages are removed. It is easy to implement with a buffer in memory.
But how are you going to get rid of the old lines if you write data directly
to the log file?
Rotation will not help you with that (we actually don't need to store such old
logs).

-- 
Regards,
Artem Senichev
Software Engineer, YADRO.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-26  8:56                     ` Artem Senichev
@ 2021-05-26 15:17                       ` Nan Zhou
  2021-05-26 16:08                         ` Artem Senichev
  0 siblings, 1 reply; 23+ messages in thread
From: Nan Zhou @ 2021-05-26 15:17 UTC (permalink / raw)
  To: Artem Senichev
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 3601 bytes --]

>
> > > > 3. zlib_file.xpp, zlib_exception.xpp:
> > > > will be removed or slightly changed; we can potentially use the linux
> > > > logrotate which has built-in compression and file rotation (in this
> case
> > > > these compression utilities will be removed).
> > > > The latest log file isn't compressed any more. History log files are
> > > > still compressed.
> > > Just curious, how are you going to remove the oldest messages from the
> > > latest file in runtime? You are not going to rewrite the entire file on
> > > every input character, are you?
> >
> > The following is my current idea: we will rename the latest file to
> > something else and notify the writer (hostlogger) to close its old file
> > descriptor and open a new one (should be doable via linux logrotate and
> > inotify or some signal handlers, as logrotate is able to send some
> signals
> > to hostlogger if a rotation is performed). The writer keeps appending
> logs
> > most of the time using the same fd unless the latest file is rotated.
> This
> > should be better than truncating the file where the reader (BMCWeb) won't
> > have race conditions (it might read old snapshots but it is not a big
> deal
> > in our case).
> Currently we can keep the last N lines of the host's output, the oldest
> messages are removed. It is easy to implement with a buffer in memory.
> But how are you going to get rid of the old lines if you write data
> directly
> to the log file?
> Rotation will not help you with that (we actually don't need to store such
> old
> logs).

We plan to implement something similar to rotate count
<https://linux.die.net/man/8/logrotate> in linux logrotate. It is basically
like a ring buffer in the file system. We keep N log files. The latest log
file is in plain text and the writer keeps appending data to it. The rest
N-1 files are compressed.

On Wed, May 26, 2021 at 1:56 AM Artem Senichev <artemsen@gmail.com> wrote:

> On Tue, May 25, 2021 at 11:51:44PM -0700, Nan Zhou wrote:
> > > > 3. zlib_file.xpp, zlib_exception.xpp:
> > > > will be removed or slightly changed; we can potentially use the linux
> > > > logrotate which has built-in compression and file rotation (in this
> case
> > > > these compression utilities will be removed).
> > > > The latest log file isn't compressed any more. History log files are
> > > > still compressed.
> > > Just curious, how are you going to remove the oldest messages from the
> > > latest file in runtime? You are not going to rewrite the entire file on
> > > every input character, are you?
> >
> > The following is my current idea: we will rename the latest file to
> > something else and notify the writer (hostlogger) to close its old file
> > descriptor and open a new one (should be doable via linux logrotate and
> > inotify or some signal handlers, as logrotate is able to send some
> signals
> > to hostlogger if a rotation is performed). The writer keeps appending
> logs
> > most of the time using the same fd unless the latest file is rotated.
> This
> > should be better than truncating the file where the reader (BMCWeb) won't
> > have race conditions (it might read old snapshots but it is not a big
> deal
> > in our case).
>
> Currently we can keep the last N lines of the host's output, the oldest
> messages are removed. It is easy to implement with a buffer in memory.
> But how are you going to get rid of the old lines if you write data
> directly
> to the log file?
> Rotation will not help you with that (we actually don't need to store such
> old
> logs).
>
> --
> Regards,
> Artem Senichev
> Software Engineer, YADRO.
>

[-- Attachment #2: Type: text/html, Size: 5500 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-26 15:17                       ` Nan Zhou
@ 2021-05-26 16:08                         ` Artem Senichev
  2021-05-26 16:20                           ` Nan Zhou
  0 siblings, 1 reply; 23+ messages in thread
From: Artem Senichev @ 2021-05-26 16:08 UTC (permalink / raw)
  To: Nan Zhou
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

On Wed, May 26, 2021 at 08:17:11AM -0700, Nan Zhou wrote:
> >
> > > > > 3. zlib_file.xpp, zlib_exception.xpp:
> > > > > will be removed or slightly changed; we can potentially use the linux
> > > > > logrotate which has built-in compression and file rotation (in this
> > case
> > > > > these compression utilities will be removed).
> > > > > The latest log file isn't compressed any more. History log files are
> > > > > still compressed.
> > > > Just curious, how are you going to remove the oldest messages from the
> > > > latest file in runtime? You are not going to rewrite the entire file on
> > > > every input character, are you?
> > >
> > > The following is my current idea: we will rename the latest file to
> > > something else and notify the writer (hostlogger) to close its old file
> > > descriptor and open a new one (should be doable via linux logrotate and
> > > inotify or some signal handlers, as logrotate is able to send some
> > signals
> > > to hostlogger if a rotation is performed). The writer keeps appending
> > logs
> > > most of the time using the same fd unless the latest file is rotated.
> > This
> > > should be better than truncating the file where the reader (BMCWeb) won't
> > > have race conditions (it might read old snapshots but it is not a big
> > deal
> > > in our case).
> > Currently we can keep the last N lines of the host's output, the oldest
> > messages are removed. It is easy to implement with a buffer in memory.
> > But how are you going to get rid of the old lines if you write data
> > directly
> > to the log file?
> > Rotation will not help you with that (we actually don't need to store such
> > old
> > logs).
> 
> We plan to implement something similar to rotate count
> <https://linux.die.net/man/8/logrotate> in linux logrotate. It is basically
> like a ring buffer in the file system. We keep N log files. The latest log
> file is in plain text and the writer keeps appending data to it. The rest
> N-1 files are compressed.

In this case, you will keep full logs without gaps:
```
Host start <- log is empty, start logging
|
[...] <- write file, compress and rotate file
|
Host reboot or shut down
```

If there are too many logs, logrotate removes the oldest one and we lose the
boot log (form host start).

This is the default Hostlogger mode:
```
Host start <- log is empty, start logging
|
[line 3000] <- flush 3000 lines to the persistent file
|
[...] <- these logs are skipped (the last 3000 lines are in memory)
|
Host reboot or shut down <- flush last 3000 lines to the file
```

-- 
Regards,
Artem Senichev
Software Engineer, YADRO.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-26 16:08                         ` Artem Senichev
@ 2021-05-26 16:20                           ` Nan Zhou
  2021-05-26 18:21                             ` Artem Senichev
  0 siblings, 1 reply; 23+ messages in thread
From: Nan Zhou @ 2021-05-26 16:20 UTC (permalink / raw)
  To: Artem Senichev
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 4201 bytes --]

>
> > We plan to implement something similar to rotate count
> > <https://linux.die.net/man/8/logrotate> in linux logrotate. It is
> basically
> > like a ring buffer in the file system. We keep N log files. The latest
> log
> > file is in plain text and the writer keeps appending data to it. The rest
> > N-1 files are compressed.
> In this case, you will keep full logs without gaps:
> ```
> Host start <- log is empty, start logging
> |
> [...] <- write file, compress and rotate file
> |
> Host reboot or shut down
> ```
> If there are too many logs, logrotate removes the oldest one and we lose
> the
> boot log (form host start).
> This is the default Hostlogger mode:
> ```
> Host start <- log is empty, start logging
> |
> [line 3000] <- flush 3000 lines to the persistent file
> |
> [...] <- these logs are skipped (the last 3000 lines are in memory)
> |
> Host reboot or shut down <- flush last 3000 lines to the file


Thanks for your explanation, but I didn't get it. Are you arguing that one
can keep more logs in memory rather than on disk? If there are too many
logs in a boot cycle, won't the current hostlogger lose some earlier logs
(boot logs) as well? Or did me missing something?

Also, we already talked about it: there's a problem that if BMC loses the
power before it sends out a signal to hostlogger, data in memory won't be
persisted.

On Wed, May 26, 2021 at 9:08 AM Artem Senichev <artemsen@gmail.com> wrote:

> On Wed, May 26, 2021 at 08:17:11AM -0700, Nan Zhou wrote:
> > >
> > > > > > 3. zlib_file.xpp, zlib_exception.xpp:
> > > > > > will be removed or slightly changed; we can potentially use the
> linux
> > > > > > logrotate which has built-in compression and file rotation (in
> this
> > > case
> > > > > > these compression utilities will be removed).
> > > > > > The latest log file isn't compressed any more. History log files
> are
> > > > > > still compressed.
> > > > > Just curious, how are you going to remove the oldest messages from
> the
> > > > > latest file in runtime? You are not going to rewrite the entire
> file on
> > > > > every input character, are you?
> > > >
> > > > The following is my current idea: we will rename the latest file to
> > > > something else and notify the writer (hostlogger) to close its old
> file
> > > > descriptor and open a new one (should be doable via linux logrotate
> and
> > > > inotify or some signal handlers, as logrotate is able to send some
> > > signals
> > > > to hostlogger if a rotation is performed). The writer keeps appending
> > > logs
> > > > most of the time using the same fd unless the latest file is rotated.
> > > This
> > > > should be better than truncating the file where the reader (BMCWeb)
> won't
> > > > have race conditions (it might read old snapshots but it is not a big
> > > deal
> > > > in our case).
> > > Currently we can keep the last N lines of the host's output, the oldest
> > > messages are removed. It is easy to implement with a buffer in memory.
> > > But how are you going to get rid of the old lines if you write data
> > > directly
> > > to the log file?
> > > Rotation will not help you with that (we actually don't need to store
> such
> > > old
> > > logs).
> >
> > We plan to implement something similar to rotate count
> > <https://linux.die.net/man/8/logrotate> in linux logrotate. It is
> basically
> > like a ring buffer in the file system. We keep N log files. The latest
> log
> > file is in plain text and the writer keeps appending data to it. The rest
> > N-1 files are compressed.
>
> In this case, you will keep full logs without gaps:
> ```
> Host start <- log is empty, start logging
> |
> [...] <- write file, compress and rotate file
> |
> Host reboot or shut down
> ```
>
> If there are too many logs, logrotate removes the oldest one and we lose
> the
> boot log (form host start).
>
> This is the default Hostlogger mode:
> ```
> Host start <- log is empty, start logging
> |
> [line 3000] <- flush 3000 lines to the persistent file
> |
> [...] <- these logs are skipped (the last 3000 lines are in memory)
> |
> Host reboot or shut down <- flush last 3000 lines to the file
> ```
>
> --
> Regards,
> Artem Senichev
> Software Engineer, YADRO.
>

[-- Attachment #2: Type: text/html, Size: 5725 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-26 16:20                           ` Nan Zhou
@ 2021-05-26 18:21                             ` Artem Senichev
  2021-05-26 19:21                               ` Nan Zhou
  0 siblings, 1 reply; 23+ messages in thread
From: Artem Senichev @ 2021-05-26 18:21 UTC (permalink / raw)
  To: Nan Zhou
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

On Wed, May 26, 2021 at 09:20:38AM -0700, Nan Zhou wrote:
> >
> > > We plan to implement something similar to rotate count
> > > <https://linux.die.net/man/8/logrotate> in linux logrotate. It is
> > basically
> > > like a ring buffer in the file system. We keep N log files. The latest
> > log
> > > file is in plain text and the writer keeps appending data to it. The rest
> > > N-1 files are compressed.
> > In this case, you will keep full logs without gaps:
> > ```
> > Host start <- log is empty, start logging
> > |
> > [...] <- write file, compress and rotate file
> > |
> > Host reboot or shut down
> > ```
> > If there are too many logs, logrotate removes the oldest one and we lose
> > the
> > boot log (form host start).
> > This is the default Hostlogger mode:
> > ```
> > Host start <- log is empty, start logging
> > |
> > [line 3000] <- flush 3000 lines to the persistent file
> > |
> > [...] <- these logs are skipped (the last 3000 lines are in memory)
> > |
> > Host reboot or shut down <- flush last 3000 lines to the file
> 
> 
> Thanks for your explanation, but I didn't get it. Are you arguing that one
> can keep more logs in memory rather than on disk?

Of course not! =)

> If there are too many logs in a boot cycle, won't the current hostlogger
> lose some earlier logs (boot logs) as well?

That's the point!
Hostlogger does not lose these logs. It writes the boot messages, then skips
the middle and writes the last 3000 lines when the host shuts down.
We have two log files per host session: start and end.
It is too expensive to store all host output, so mid-session messages are
skipped.
It can be easily implemented with a buffer, but I am not sure we can achieve
this with logrotate.

> Or did me missing something?
> 
> Also, we already talked about it: there's a problem that if BMC loses the
> power before it sends out a signal to hostlogger, data in memory won't be
> persisted.

Yes, I agree that this is a problem. But there are ways to fix it without
breaking the current functionality of Hostlogger.
We can use rsyslog with external log server, or increase the buffer size
in obmc-console-server, or use systemd-cat with logrotate.
We can even add a new mode to Hostlogger that will not use the buffer, but
as I said earlier, there are not many common parts. 

-- 
Regards,
Artem Senichev
Software Engineer, YADRO.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-26 18:21                             ` Artem Senichev
@ 2021-05-26 19:21                               ` Nan Zhou
  2021-06-11 18:31                                 ` Ed Tanous
  0 siblings, 1 reply; 23+ messages in thread
From: Nan Zhou @ 2021-05-26 19:21 UTC (permalink / raw)
  To: Artem Senichev
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 4366 bytes --]

>
> > If there are too many logs in a boot cycle, won't the current hostlogger
> > lose some earlier logs (boot logs) as well?
> That's the point!
> Hostlogger does not lose these logs. It writes the boot messages, then
> skips
> the middle and writes the last 3000 lines when the host shuts down.
> We have two log files per host session: start and end.
> It is too expensive to store all host output, so mid-session messages are
> skipped.
> It can be easily implemented with a buffer, but I am not sure we can
> achieve
> this with logrotate.

Thanks for the information. I am not aware of this functionality in the
current hostlogger. Are you saying it will be implemented in the future or
I miss it in the current codes.
One of our options for log rotations is writing our own codes, I am sure we
can implement the logic you mentioned above without too much effort.
The linux logrotate also has "prerotate scripts", we can carefully name the
compressed log file and keep the oldest several ones (which have host boot
logs) out of rotation.

> Or did me missing something?
> >
> > Also, we already talked about it: there's a problem that if BMC loses the
> > power before it sends out a signal to hostlogger, data in memory won't be
> > persisted.
> Yes, I agree that this is a problem. But there are ways to fix it without
> breaking the current functionality of Hostlogger.
> We can use rsyslog with external log server, or increase the buffer size
> in obmc-console-server, or use systemd-cat with logrotate.
> We can even add a new mode to Hostlogger that will not use the buffer, but
> as I said earlier, there are not many common parts.

I guess you are arguing we need a new daemon rather than modify Hostlogger,
right? +Ed Tanous <edtanous@google.com> here to see what his opinion is.

On Wed, May 26, 2021 at 11:22 AM Artem Senichev <artemsen@gmail.com> wrote:

> On Wed, May 26, 2021 at 09:20:38AM -0700, Nan Zhou wrote:
> > >
> > > > We plan to implement something similar to rotate count
> > > > <https://linux.die.net/man/8/logrotate> in linux logrotate. It is
> > > basically
> > > > like a ring buffer in the file system. We keep N log files. The
> latest
> > > log
> > > > file is in plain text and the writer keeps appending data to it. The
> rest
> > > > N-1 files are compressed.
> > > In this case, you will keep full logs without gaps:
> > > ```
> > > Host start <- log is empty, start logging
> > > |
> > > [...] <- write file, compress and rotate file
> > > |
> > > Host reboot or shut down
> > > ```
> > > If there are too many logs, logrotate removes the oldest one and we
> lose
> > > the
> > > boot log (form host start).
> > > This is the default Hostlogger mode:
> > > ```
> > > Host start <- log is empty, start logging
> > > |
> > > [line 3000] <- flush 3000 lines to the persistent file
> > > |
> > > [...] <- these logs are skipped (the last 3000 lines are in memory)
> > > |
> > > Host reboot or shut down <- flush last 3000 lines to the file
> >
> >
> > Thanks for your explanation, but I didn't get it. Are you arguing that
> one
> > can keep more logs in memory rather than on disk?
>
> Of course not! =)
>
> > If there are too many logs in a boot cycle, won't the current hostlogger
> > lose some earlier logs (boot logs) as well?
>
> That's the point!
> Hostlogger does not lose these logs. It writes the boot messages, then
> skips
> the middle and writes the last 3000 lines when the host shuts down.
> We have two log files per host session: start and end.
> It is too expensive to store all host output, so mid-session messages are
> skipped.
> It can be easily implemented with a buffer, but I am not sure we can
> achieve
> this with logrotate.
>
> > Or did me missing something?
> >
> > Also, we already talked about it: there's a problem that if BMC loses the
> > power before it sends out a signal to hostlogger, data in memory won't be
> > persisted.
>
> Yes, I agree that this is a problem. But there are ways to fix it without
> breaking the current functionality of Hostlogger.
> We can use rsyslog with external log server, or increase the buffer size
> in obmc-console-server, or use systemd-cat with logrotate.
> We can even add a new mode to Hostlogger that will not use the buffer, but
> as I said earlier, there are not many common parts.
>
> --
> Regards,
> Artem Senichev
> Software Engineer, YADRO.
>

[-- Attachment #2: Type: text/html, Size: 6154 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-05-26 19:21                               ` Nan Zhou
@ 2021-06-11 18:31                                 ` Ed Tanous
  2021-06-11 21:07                                   ` Nan Zhou
  0 siblings, 1 reply; 23+ messages in thread
From: Ed Tanous @ 2021-06-11 18:31 UTC (permalink / raw)
  To: Nan Zhou
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	a.senichev, Richard Hanley, Artem Senichev, a.filippov

On Wed, May 26, 2021 at 12:21 PM Nan Zhou <nanzhou@google.com> wrote:
>>
>> > If there are too many logs in a boot cycle, won't the current hostlogger
>> > lose some earlier logs (boot logs) as well?
>> That's the point!
>> Hostlogger does not lose these logs. It writes the boot messages, then skips
>> the middle and writes the last 3000 lines when the host shuts down.
>> We have two log files per host session: start and end.
>> It is too expensive to store all host output, so mid-session messages are
>> skipped.
>> It can be easily implemented with a buffer, but I am not sure we can achieve
>> this with logrotate.
>
> Thanks for the information. I am not aware of this functionality in the current hostlogger. Are you saying it will be implemented in the future or I miss it in the current codes.
> One of our options for log rotations is writing our own codes, I am sure we can implement the logic you mentioned above without too much effort.
> The linux logrotate also has "prerotate scripts", we can carefully name the compressed log file and keep the oldest several ones (which have host boot logs) out of rotation.

For what it's worth, I don't know if we have to use logrotate
as-written if it's not a good fit here, but that style of writing to
disk makes for easily tail-able logs, and has pretty well defined
behavior for log rotation.  If we could keep the behavior (or the
behavior + rotating on boot events), even if we didn't use logrotate
itself, I think it would be a benefit to this.

>
>> > Or did me missing something?
>> >
>> > Also, we already talked about it: there's a problem that if BMC loses the
>> > power before it sends out a signal to hostlogger, data in memory won't be
>> > persisted.
>> Yes, I agree that this is a problem. But there are ways to fix it without
>> breaking the current functionality of Hostlogger.
>> We can use rsyslog with external log server, or increase the buffer size
>> in obmc-console-server, or use systemd-cat with logrotate.
>> We can even add a new mode to Hostlogger that will not use the buffer, but
>> as I said earlier, there are not many common parts.
>
> I guess you are arguing we need a new daemon rather than modify Hostlogger, right? +Ed Tanous here to see what his opinion is.

If Nans use case is in fact totally different, and can't be handled in
the same application I think that's ok, but I'd like to see the new
application put in the hostlogger repo so it can share the routines
that are common (things like finding and opening the unix socket,
managing the reads, and the zlib integration) and to ensure that users
find it when searching for code that solves this problem, as on the
outset they're pretty similar, just with seemingly different rules.
If we don't put it there, it seems like we'd have to duplicate a lot
of code.

>
> On Wed, May 26, 2021 at 11:22 AM Artem Senichev <artemsen@gmail.com> wrote:
>>
>> On Wed, May 26, 2021 at 09:20:38AM -0700, Nan Zhou wrote:
>> > >
>> > > > We plan to implement something similar to rotate count
>> > > > <https://linux.die.net/man/8/logrotate> in linux logrotate. It is
>> > > basically
>> > > > like a ring buffer in the file system. We keep N log files. The latest
>> > > log
>> > > > file is in plain text and the writer keeps appending data to it. The rest
>> > > > N-1 files are compressed.
>> > > In this case, you will keep full logs without gaps:
>> > > ```
>> > > Host start <- log is empty, start logging
>> > > |
>> > > [...] <- write file, compress and rotate file
>> > > |
>> > > Host reboot or shut down
>> > > ```
>> > > If there are too many logs, logrotate removes the oldest one and we lose
>> > > the
>> > > boot log (form host start).
>> > > This is the default Hostlogger mode:
>> > > ```
>> > > Host start <- log is empty, start logging
>> > > |
>> > > [line 3000] <- flush 3000 lines to the persistent file
>> > > |
>> > > [...] <- these logs are skipped (the last 3000 lines are in memory)
>> > > |
>> > > Host reboot or shut down <- flush last 3000 lines to the file
>> >
>> >
>> > Thanks for your explanation, but I didn't get it. Are you arguing that one
>> > can keep more logs in memory rather than on disk?
>>
>> Of course not! =)
>>
>> > If there are too many logs in a boot cycle, won't the current hostlogger
>> > lose some earlier logs (boot logs) as well?
>>
>> That's the point!
>> Hostlogger does not lose these logs. It writes the boot messages, then skips
>> the middle and writes the last 3000 lines when the host shuts down.
>> We have two log files per host session: start and end.
>> It is too expensive to store all host output, so mid-session messages are
>> skipped.
>> It can be easily implemented with a buffer, but I am not sure we can achieve
>> this with logrotate.
>>
>> > Or did me missing something?
>> >
>> > Also, we already talked about it: there's a problem that if BMC loses the
>> > power before it sends out a signal to hostlogger, data in memory won't be
>> > persisted.
>>
>> Yes, I agree that this is a problem. But there are ways to fix it without
>> breaking the current functionality of Hostlogger.
>> We can use rsyslog with external log server, or increase the buffer size
>> in obmc-console-server, or use systemd-cat with logrotate.
>> We can even add a new mode to Hostlogger that will not use the buffer, but
>> as I said earlier, there are not many common parts.
>>
>> --
>> Regards,
>> Artem Senichev
>> Software Engineer, YADRO.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-06-11 18:31                                 ` Ed Tanous
@ 2021-06-11 21:07                                   ` Nan Zhou
  2021-06-14  6:18                                     ` Artem Senichev
  0 siblings, 1 reply; 23+ messages in thread
From: Nan Zhou @ 2021-06-11 21:07 UTC (permalink / raw)
  To: Ed Tanous
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	a.senichev, Richard Hanley, Artem Senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 7780 bytes --]

>
> >
> >> > Or did me missing something?
> >> >
> >> > Also, we already talked about it: there's a problem that if BMC loses
> the
> >> > power before it sends out a signal to hostlogger, data in memory
> won't be
> >> > persisted.
> >> Yes, I agree that this is a problem. But there are ways to fix it
> without
> >> breaking the current functionality of Hostlogger.
> >> We can use rsyslog with external log server, or increase the buffer size
> >> in obmc-console-server, or use systemd-cat with logrotate.
> >> We can even add a new mode to Hostlogger that will not use the buffer,
> but
> >> as I said earlier, there are not many common parts.
> >
> > I guess you are arguing we need a new daemon rather than modify
> Hostlogger, right? +Ed Tanous here to see what his opinion is.
> If Nans use case is in fact totally different, and can't be handled in
> the same application I think that's ok, but I'd like to see the new
> application put in the hostlogger repo so it can share the routines
> that are common (things like finding and opening the unix socket,
> managing the reads, and the zlib integration) and to ensure that users
> find it when searching for code that solves this problem, as on the
> outset they're pretty similar, just with seemingly different rules.
> If we don't put it there, it seems like we'd have to duplicate a lot
> of code.

Thanks a lot for the previous discussion. Following up on this, is it
acceptable if we add a new application (it will be a new binary + a new
systemd service config) in the hostlogger repo to support the stream
application? We can add options to control whether to turn on this new
stream application. In this way, existing applications stay unchanged. As
Ed said, we could use codes that deal with unix sockets and reads. It
should also make sense that console codes are put together in the
same place.

Please let us know what you think. Your confirmation will be very valuable
and will unblock our development.

On Fri, Jun 11, 2021 at 11:31 AM Ed Tanous <edtanous@google.com> wrote:

> On Wed, May 26, 2021 at 12:21 PM Nan Zhou <nanzhou@google.com> wrote:
> >>
> >> > If there are too many logs in a boot cycle, won't the current
> hostlogger
> >> > lose some earlier logs (boot logs) as well?
> >> That's the point!
> >> Hostlogger does not lose these logs. It writes the boot messages, then
> skips
> >> the middle and writes the last 3000 lines when the host shuts down.
> >> We have two log files per host session: start and end.
> >> It is too expensive to store all host output, so mid-session messages
> are
> >> skipped.
> >> It can be easily implemented with a buffer, but I am not sure we can
> achieve
> >> this with logrotate.
> >
> > Thanks for the information. I am not aware of this functionality in the
> current hostlogger. Are you saying it will be implemented in the future or
> I miss it in the current codes.
> > One of our options for log rotations is writing our own codes, I am sure
> we can implement the logic you mentioned above without too much effort.
> > The linux logrotate also has "prerotate scripts", we can carefully name
> the compressed log file and keep the oldest several ones (which have host
> boot logs) out of rotation.
>
> For what it's worth, I don't know if we have to use logrotate
> as-written if it's not a good fit here, but that style of writing to
> disk makes for easily tail-able logs, and has pretty well defined
> behavior for log rotation.  If we could keep the behavior (or the
> behavior + rotating on boot events), even if we didn't use logrotate
> itself, I think it would be a benefit to this.
>
> >
> >> > Or did me missing something?
> >> >
> >> > Also, we already talked about it: there's a problem that if BMC loses
> the
> >> > power before it sends out a signal to hostlogger, data in memory
> won't be
> >> > persisted.
> >> Yes, I agree that this is a problem. But there are ways to fix it
> without
> >> breaking the current functionality of Hostlogger.
> >> We can use rsyslog with external log server, or increase the buffer size
> >> in obmc-console-server, or use systemd-cat with logrotate.
> >> We can even add a new mode to Hostlogger that will not use the buffer,
> but
> >> as I said earlier, there are not many common parts.
> >
> > I guess you are arguing we need a new daemon rather than modify
> Hostlogger, right? +Ed Tanous here to see what his opinion is.
>
> If Nans use case is in fact totally different, and can't be handled in
> the same application I think that's ok, but I'd like to see the new
> application put in the hostlogger repo so it can share the routines
> that are common (things like finding and opening the unix socket,
> managing the reads, and the zlib integration) and to ensure that users
> find it when searching for code that solves this problem, as on the
> outset they're pretty similar, just with seemingly different rules.
> If we don't put it there, it seems like we'd have to duplicate a lot
> of code.
>
> >
> > On Wed, May 26, 2021 at 11:22 AM Artem Senichev <artemsen@gmail.com>
> wrote:
> >>
> >> On Wed, May 26, 2021 at 09:20:38AM -0700, Nan Zhou wrote:
> >> > >
> >> > > > We plan to implement something similar to rotate count
> >> > > > <https://linux.die.net/man/8/logrotate> in linux logrotate. It is
> >> > > basically
> >> > > > like a ring buffer in the file system. We keep N log files. The
> latest
> >> > > log
> >> > > > file is in plain text and the writer keeps appending data to it.
> The rest
> >> > > > N-1 files are compressed.
> >> > > In this case, you will keep full logs without gaps:
> >> > > ```
> >> > > Host start <- log is empty, start logging
> >> > > |
> >> > > [...] <- write file, compress and rotate file
> >> > > |
> >> > > Host reboot or shut down
> >> > > ```
> >> > > If there are too many logs, logrotate removes the oldest one and we
> lose
> >> > > the
> >> > > boot log (form host start).
> >> > > This is the default Hostlogger mode:
> >> > > ```
> >> > > Host start <- log is empty, start logging
> >> > > |
> >> > > [line 3000] <- flush 3000 lines to the persistent file
> >> > > |
> >> > > [...] <- these logs are skipped (the last 3000 lines are in memory)
> >> > > |
> >> > > Host reboot or shut down <- flush last 3000 lines to the file
> >> >
> >> >
> >> > Thanks for your explanation, but I didn't get it. Are you arguing
> that one
> >> > can keep more logs in memory rather than on disk?
> >>
> >> Of course not! =)
> >>
> >> > If there are too many logs in a boot cycle, won't the current
> hostlogger
> >> > lose some earlier logs (boot logs) as well?
> >>
> >> That's the point!
> >> Hostlogger does not lose these logs. It writes the boot messages, then
> skips
> >> the middle and writes the last 3000 lines when the host shuts down.
> >> We have two log files per host session: start and end.
> >> It is too expensive to store all host output, so mid-session messages
> are
> >> skipped.
> >> It can be easily implemented with a buffer, but I am not sure we can
> achieve
> >> this with logrotate.
> >>
> >> > Or did me missing something?
> >> >
> >> > Also, we already talked about it: there's a problem that if BMC loses
> the
> >> > power before it sends out a signal to hostlogger, data in memory
> won't be
> >> > persisted.
> >>
> >> Yes, I agree that this is a problem. But there are ways to fix it
> without
> >> breaking the current functionality of Hostlogger.
> >> We can use rsyslog with external log server, or increase the buffer size
> >> in obmc-console-server, or use systemd-cat with logrotate.
> >> We can even add a new mode to Hostlogger that will not use the buffer,
> but
> >> as I said earlier, there are not many common parts.
> >>
> >> --
> >> Regards,
> >> Artem Senichev
> >> Software Engineer, YADRO.
>

[-- Attachment #2: Type: text/html, Size: 10627 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-06-11 21:07                                   ` Nan Zhou
@ 2021-06-14  6:18                                     ` Artem Senichev
  2021-06-18 18:18                                       ` Nan Zhou
  0 siblings, 1 reply; 23+ messages in thread
From: Artem Senichev @ 2021-06-14  6:18 UTC (permalink / raw)
  To: Nan Zhou
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

On Sat, Jun 12, 2021 at 12:07 AM Nan Zhou <nanzhou@google.com> wrote:
>>
>> >
>> >> > Or did me missing something?
>> >> >
>> >> > Also, we already talked about it: there's a problem that if BMC loses the
>> >> > power before it sends out a signal to hostlogger, data in memory won't be
>> >> > persisted.
>> >> Yes, I agree that this is a problem. But there are ways to fix it without
>> >> breaking the current functionality of Hostlogger.
>> >> We can use rsyslog with external log server, or increase the buffer size
>> >> in obmc-console-server, or use systemd-cat with logrotate.
>> >> We can even add a new mode to Hostlogger that will not use the buffer, but
>> >> as I said earlier, there are not many common parts.
>> >
>> > I guess you are arguing we need a new daemon rather than modify Hostlogger, right? +Ed Tanous here to see what his opinion is.
>> If Nans use case is in fact totally different, and can't be handled in
>> the same application I think that's ok, but I'd like to see the new
>> application put in the hostlogger repo so it can share the routines
>> that are common (things like finding and opening the unix socket,
>> managing the reads, and the zlib integration) and to ensure that users
>> find it when searching for code that solves this problem, as on the
>> outset they're pretty similar, just with seemingly different rules.
>> If we don't put it there, it seems like we'd have to duplicate a lot
>> of code.
>
> Thanks a lot for the previous discussion. Following up on this, is it acceptable if we add a new application (it will be a new binary + a new systemd service config) in the hostlogger repo to support the stream application? We can add options to control whether to turn on this new stream application. In this way, existing applications stay unchanged. As Ed said, we could use codes that deal with unix sockets and reads. It should also make sense that console codes are put together in the same place.
>
> Please let us know what you think. Your confirmation will be very valuable and will unblock our development.

I still don't see a reason to have two different applications in one
repository, but let's try.
I believe that the common part with socket opening/reading (about 100
lines of code) is nothing compared to the complexity of supporting
such sophisticated configuration in meson and bb files.

--
Best regards,
Artem Senichev

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Link phosphor-hostlogger and bmcweb
  2021-06-14  6:18                                     ` Artem Senichev
@ 2021-06-18 18:18                                       ` Nan Zhou
  0 siblings, 0 replies; 23+ messages in thread
From: Nan Zhou @ 2021-06-18 18:18 UTC (permalink / raw)
  To: Artem Senichev
  Cc: Spencer Ku, Litzung Chen, OpenBMC Maillist, Alexander Amelkin,
	Ed Tanous, Richard Hanley, a.senichev, a.filippov

[-- Attachment #1: Type: text/plain, Size: 3265 bytes --]

Hi Artem,

I still don't see a reason to have two different applications in one
> repository, but let's try.
> I believe that the common part with socket opening/reading (about 100
> lines of code) is nothing compared to the complexity of supporting
> such sophisticated configuration in meson and bb files.


Please take a look at
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-hostlogger/+/44241. I
made the HostLogger build the same binary but with different behaviour. In
this way, no bb file changes are needed. It leverages existing socket
reading, config reading, and dbus event loop. By default it doesn't affect
the existing buffer based HostLogger at all.

Sincerely,
Nan


On Sun, Jun 13, 2021 at 11:18 PM Artem Senichev <artemsen@gmail.com> wrote:

> On Sat, Jun 12, 2021 at 12:07 AM Nan Zhou <nanzhou@google.com> wrote:
> >>
> >> >
> >> >> > Or did me missing something?
> >> >> >
> >> >> > Also, we already talked about it: there's a problem that if BMC
> loses the
> >> >> > power before it sends out a signal to hostlogger, data in memory
> won't be
> >> >> > persisted.
> >> >> Yes, I agree that this is a problem. But there are ways to fix it
> without
> >> >> breaking the current functionality of Hostlogger.
> >> >> We can use rsyslog with external log server, or increase the buffer
> size
> >> >> in obmc-console-server, or use systemd-cat with logrotate.
> >> >> We can even add a new mode to Hostlogger that will not use the
> buffer, but
> >> >> as I said earlier, there are not many common parts.
> >> >
> >> > I guess you are arguing we need a new daemon rather than modify
> Hostlogger, right? +Ed Tanous here to see what his opinion is.
> >> If Nans use case is in fact totally different, and can't be handled in
> >> the same application I think that's ok, but I'd like to see the new
> >> application put in the hostlogger repo so it can share the routines
> >> that are common (things like finding and opening the unix socket,
> >> managing the reads, and the zlib integration) and to ensure that users
> >> find it when searching for code that solves this problem, as on the
> >> outset they're pretty similar, just with seemingly different rules.
> >> If we don't put it there, it seems like we'd have to duplicate a lot
> >> of code.
> >
> > Thanks a lot for the previous discussion. Following up on this, is it
> acceptable if we add a new application (it will be a new binary + a new
> systemd service config) in the hostlogger repo to support the stream
> application? We can add options to control whether to turn on this new
> stream application. In this way, existing applications stay unchanged. As
> Ed said, we could use codes that deal with unix sockets and reads. It
> should also make sense that console codes are put together in the same
> place.
> >
> > Please let us know what you think. Your confirmation will be very
> valuable and will unblock our development.
>
> I still don't see a reason to have two different applications in one
> repository, but let's try.
> I believe that the common part with socket opening/reading (about 100
> lines of code) is nothing compared to the complexity of supporting
> such sophisticated configuration in meson and bb files.
>
> --
> Best regards,
> Artem Senichev
>

[-- Attachment #2: Type: text/html, Size: 4329 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2021-06-18 18:18 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-20 23:29 Link phosphor-hostlogger and bmcweb Nan Zhou
2021-05-21  6:10 ` Artem Senichev
2021-05-21 15:10   ` Nan Zhou
2021-05-21 17:25     ` Ed Tanous
2021-05-21 17:23   ` Ed Tanous
2021-05-21 17:51     ` Nan Zhou
2021-05-24  7:52       ` Artem Senichev
2021-05-24 16:27         ` Ed Tanous
2021-05-25  6:41           ` Artem Senichev
2021-05-25 15:03             ` Nan Zhou
2021-05-26  5:29               ` Nan Zhou
2021-05-26  6:11                 ` Artem Senichev
2021-05-26  6:51                   ` Nan Zhou
2021-05-26  8:56                     ` Artem Senichev
2021-05-26 15:17                       ` Nan Zhou
2021-05-26 16:08                         ` Artem Senichev
2021-05-26 16:20                           ` Nan Zhou
2021-05-26 18:21                             ` Artem Senichev
2021-05-26 19:21                               ` Nan Zhou
2021-06-11 18:31                                 ` Ed Tanous
2021-06-11 21:07                                   ` Nan Zhou
2021-06-14  6:18                                     ` Artem Senichev
2021-06-18 18:18                                       ` Nan Zhou

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.