openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Paul Fertser <fercerpav@gmail.com>
To: Ed Tanous <ed@tanous.net>
Cc: Zhikui Ren <zhikui.ren@intel.com>,
	Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>,
	Andrei Tutolmin <a.tutolmin@gagar-in.com>,
	Vernon Mauery <vernon.mauery@linux.intel.com>,
	OpenBMC Maillist <openbmc@lists.ozlabs.org>,
	Timofei Nikitin <t.nikitin@gagar-in.com>,
	Jason Bills <jason.m.bills@linux.intel.com>,
	Gunnar Mills <gmills@linux.vnet.ibm.com>
Subject: Re: Redfish requests for dbus-sensors data are needlessly slow - analysis
Date: Thu, 29 Jul 2021 18:29:19 +0300	[thread overview]
Message-ID: <20210729152919.GX875@home.paul.comp> (raw)
In-Reply-To: <CACWQX81MBkR3JVRDGbJJMzgGgb3E03HfZTtKu_c0m51g4hXtoA@mail.gmail.com>

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@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@gmail.com

  reply	other threads:[~2021-07-29 15:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29  9:46 Redfish requests for dbus-sensors data are needlessly slow - analysis Paul Fertser
2021-07-29 14:53 ` Ed Tanous
2021-07-29 15:29   ` Paul Fertser [this message]
2021-07-29 16:08     ` Ed Tanous
2021-08-04 10:41       ` Paul Fertser
2021-08-04 18:51         ` Ed Tanous
2021-08-05 18:57           ` Paul Fertser
2021-08-05 19:25             ` Ed Tanous
2021-08-03  3:58   ` Andrew Jeffery

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210729152919.GX875@home.paul.comp \
    --to=fercerpav@gmail.com \
    --cc=a.tutolmin@gagar-in.com \
    --cc=ed@tanous.net \
    --cc=gmills@linux.vnet.ibm.com \
    --cc=jae.hyun.yoo@linux.intel.com \
    --cc=jason.m.bills@linux.intel.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=t.nikitin@gagar-in.com \
    --cc=vernon.mauery@linux.intel.com \
    --cc=zhikui.ren@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).