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

[Paul Fertser]

Hello Ed,

Thank you for the reply.

On Thu, Jul 29, 2021 at 07:53:38AM -0700, Ed Tanous wrote:
> On Thu, Jul 29, 2021 at 2:46 AM Paul Fertser <fercerpav at gmail.com> wrote:
> > Here follows my naive exploratory patch. It works for the sensors
> > we're interested in (unless they appear after the first Redfish
> > request made to bmcweb as the cache for the expensive D-Bus calls is
> > never invalidated). It doesn't handle complex inventory associations.
> 
> The "unless" is kind of important here.  Sensors can show up or be
> removed at any time, especially on an entity-manager enabled system.

So bmcweb can subscribe to the signals from entity-manager and
invalidate the caches just in case if any EM event registered.

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

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

> > I would be willing to work on doing the right thing but for that I'm
> > lacking the understanding of the complete picture involved in handling
> > all types of sensors and interfaces, so I'm kindly asking for your
> > help with it.
> >
> > diff --git a/redfish-core/lib/sensors.hpp b/redfish-core/lib/sensors.hpp
> 
> Can you please submit the below to gerrit as a WIP instead.  There's
> much better tooling there for reviewing and testing patches.  I can
> review it more there.

Sure, will do. I didn't mean to offer it as a solution, it's just to
prove my understanding of where the bottlenecks are was about correct.

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

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