Redfish requests for dbus-sensors data are needlessly slow - analysis

[Paul Fertser]

Hello Ed,

So as promised I sent the patch to Gerrit on Friday,
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/45404 and you
were automatically added as a Reviewer but I haven't worked with WiP
before so I have no idea if you got a notification or not (I didn't
press the START REVIEW button). I'm ready to provide a documentation
patch to the "OpenBMC beginners" instructions once I learn how exactly
WiP patches are usually employed by the community.

The build is not happy with it as I didn't use clang-format
conciously, the diff before it is much clearer (understandble) in this
particular case.

On Thu, Jul 29, 2021 at 09:08:30AM -0700, Ed Tanous wrote:
> On Thu, Jul 29, 2021 at 8:29 AM Paul Fertser <fercerpav at gmail.com> wrote:
> > So bmcweb can subscribe to the signals from entity-manager and
> > invalidate the caches just in case if any EM event registered.
> 
> You have to monitor signals from all connections.  bmcweb doesn't only
> run against entity-manager.  FWIW, what you described is essentially
> entirely what the mapper does;

See how little clue I have, that's exactly the reason I'm addressing
you and the other maintainers because I'm still unable to wrap my head
around all the EM and mapper interactions and what it really takes
_and why_ to get essential information about the system health.

> > > As a general rule, bmcweb has avoided having a cache at all.  There
> > > have been several attempts at adding one, but most failed because of
> > > the same thing you found: cache invalidation is hard.
> >
> > Even if it's just dropped on any ObjectMapper or EntityManager signal?
> 
> Those aren't the only signals you need to monitor.  If you're
> interested in going down this route, I'd start by looking at the
> mapper implementation.  You'd also have to handle connections,
> disconnections, and other stuff.  At some point, what you've done
> starts to look a lot like the mapper.

Understood. So if we have the mapper already why isn't it enough to
issue just one call to it, process the reply, and then request
directly the data bmcweb needs?

> > > If we're going to implement a cache, I'd really like to see it done
> > > at a higher level (somewhere in the connection class) and done more
> > > generically than this is here, which would ensure that all endpoints
> > > are sped up, not just sensors.  With that said, if this really
> > > solves the problem and solves it well, I can't very well ignore it.
> >
> > But is it really a problem? I'm still not sure; probably it's just
> > that we invented a weird usecase not worth optimising for? Have the
> > others faced it and do they consider it to be a problem?
> 
> It's for sure a problem, although your 4 second response times sound
> worse than most.

It's usually about 2 seconds, and with my silly caching I cut it down
to half a second (four times).

> On an ast2500 I've normally seen a 700ms (ish) response time, but
> depending on how many sensors you have, and how fast the backends
> are for each one, I could easily see it getting up there pretty
> easily.

PSUSensor backend is caching hwmon values so it's reasonably fast. And
in my first mail I provided detailed timings of each step bmcweb
performs, so it's clear it doesn't add unreasonably much.

The system in question (Tioga Pass) shows 153 sensors (with dual
26-cores Xeons running).

> Just one thing to double check;  You don't have any custom sensor
> daemons running on your system do you?  Ideally everything should be
> implementing object manager.  If it doesn't, the code paths to build
> the sensor schema has to take the very slow path (ie calling Get on
> each property individually).

In my first mail on the topic I detailed all the calls, and AFAICT
there's nothing custom and no Get requests at all. Can I be missing
anything?
 
> > My real point is that apparently bmcweb is (in some cases) requesting
> > way too much data in way too many calls, using only a tiny fraction of
> > the obtained values in the end. That's why I made that lengthy
> > description of how data for a single sensor is obtained. Caching can
> > hide this to a degree, but shouldn't the root cause be fixed as well?
> 
> Yes, we should come at it from both angles, because a cache will
> still have to invalidate quite a bit, and nobody likes a bimodal
> distribution of response times.

This sounds cool but I'm not sure I understand the point. We face
caching everywhere, and it's usually multi-layered, so when
appropriately benchmarked we often see tri- and more modal
distributions. And when not building a hard real-time system the
median is much more important than the modality I guess?

> The next thing on my personal list is to get a new version of
> GetSubtree added to the mapper, that accepts a list of optional
> interfaces as well as required.  I talked about this a while back on
> discord.

The d-word offences me. Their ToS (and specifically the explicit
prohibition of "third-party" clients) make it outright absolutely
unacceptable to use the (mis)service.

> This might clean up a couple of the extra calls in the
> sensor code, but the primary target was inventory initially.

I'm still rather puzzled by the relationship between different
sensors/entities/inventory/leds/etc. Is there a document describing
everything that's currently essential?

> In a perfect world where software is easy and architectures are
> beautiful, ideally I'd like to see most redfish calls go:
> Call mapper once with a list of interfaces;  Find all connections and
> objects that implement this interface.
> Loop over connections, calling GetManagedObjects on each to read in
> the data and write directly to redfish tree.

So does anyone know what exactly is not perfect about the current
world that makes bmcweb so convoluted?

> > Does bmcweb really _need_ to do all the calls mentioned? I think the
> > fact it performs two identical D-Bus calls in a row gives a clear "no"
> > answer.
> 
> You are almost certainly right, two identical calls is wasteful, and
> we should clean that up if we can.  I'm certain there's probably
> several speedups to be gained by someone understanding the sensor code
> and optimizing it a little.

I'm happy I managed to get my point through finally.

> > After reading my outline of the process can you tell that's
> > exactly the way the OpenBMC D-Bus API is designed to be used?
> 
> I'm not sure I quite follow this question.  The sensor code you're
> looking at evolved significantly over time into the behemoth you see
> today.  It quite likely has several anti-patterns and duplicated calls
> that can be consolidated, but that didn't really show up in the
> individual diffs as features were added.

That's perfectly understandable, happens to every software projects
that's not stale. Not only software even, our own bodies have some
rather "interesting" solutions too, that's a natural result of an
evolution.

> I'm really happy someone has gone through the full flow.  Ideally we
> can start cleaning up the obvious stuff (duplicated calls, ect) in
> some patchsets.

Since I do not understand the role of /inventory/ pathes, and I do not
understand how LEDs get involved (apparently we do not have any LEDs
like that in our Tioga Pass configuration) and I didn't even try to
dig the "power" nodes I'm certainly not in a position to offer a
worthy code contribution in this area :( (other than fixing the
duplicated call which can trivially be done by passing an extra
argument; but that alone isn't fruitful much)

I very much hoped for more involvement from the bmcweb maintainers, as
solving this issue properly, in a way that doesn't break any usecases
obviously requires plenty of domain-specific knowledge (domain here
being the extensive OpenBMC D-Bus API, specifically the
sensors/inventory part of it).

Guess I'm at fault here and doing something wrong, and it's not the
first time I fail to understand how to interact with the
community. E.g. I'm still waiting for an answer to a straightforward
question asked in
https://lists.ozlabs.org/pipermail/openbmc/2021-July/027001.html
. What am I doing wrong? Am I copying the wrong people? Am I asking at
the wrong place? Should I be expecting something else entirely? I have
some reasonable experience interacting with other Free Software
communities but somehow it doesn't seem to be applicable here?

Thank you for your time.

-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav at gmail.com