Date: Wed, 10 May 2017 14:10:25 -0700
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Mathieu Fenniak <mathieu.fenniak@replicon.com>,
	pgsql-general@postgresql.org
Subject: Re: Logical decoding CPU-bound w/ large number of tables
Message-ID: <20170510211025.scae63m476ftv7i3@alap3.anarazel.de>
References: 
 <CAHoiPjzea6N0zuCi=+f9v_j94nfsy6y8SU7-=bp4=7qw6_i=Rg@mail.gmail.com>
 <20170506012055.hhmx5hirj2yk7d3g@alap3.anarazel.de>
 <19256.1494034347@sss.pgh.pa.us>
 <20170506013810.2og6vax6a2u3rhjc@alap3.anarazel.de>
 <CAHoiPjwSEHKdWkL0cz1usioc9MQPuTB8a=vbtVy=JLpuAm4Axw@mail.gmail.com>
 <12184.1494450131@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <12184.1494450131@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-general-owner@postgresql.org

Hi Tom, Mathieu,

On 2017-05-10 17:02:11 -0400, Tom Lane wrote:
> Mathieu Fenniak <mathieu.fenniak@replicon.com> writes:
> > Andres, it seems like the problem is independent of having large data
> > manipulations mixed with schema changes.  The below test case demonstrates
> > it with just schema changes.
> 
> > Tom, I've tested your patch, and it seems to have a positive impact for
> > sure.  I've documented a test case to reproduce this issue (below), and
> > your avoid-search-in-catcache-invalidate.patch reduces the test case time
> > from 63 seconds per run to 27 seconds per run.
> 
> Thanks for the test case.

Indeed!


> +   59.28%    59.05%        150349  postmaster       postgres                                      [.] hash_seq_search
> +    8.40%     8.36%         21250  postmaster       postgres                                      [.] CallSyscacheCallbacks
> +    6.37%     6.34%         16115  postmaster       postgres                                      [.] LocalExecuteInvalidationMessage
> +    5.69%     5.67%         14418  postmaster       postgres                                      [.] CatCacheInvalidate
> +    3.14%     3.12%          7942  postmaster       postgres                                      [.] SysCacheInvalidate
> +    1.72%     1.71%          4354  postmaster       postgres                                      [.] ReorderBufferCommit
> +    1.37%     1.33%          3512  postmaster       postgres                                      [.] hash_search_with_hash_value
> +    1.15%     1.15%          2913  postmaster       postgres                                      [.] InvalidateCatalogSnapshot
> 
> I looked at the hash_seq_search time a bit more, and realized that
> actually there's a pretty easy fix for that, which is to reduce
> the initial size of RelfilenodeMapHash from 1024 entries to 64.

Heh, that's not a bad idea. Given the table resizes automatically, there
seems little reason not to go there.

> You could quibble about where to set that exactly, but 1024 is
> just way too many --- in your example, there are never more than
> 5 entries in the hash, despite the presence of 10000 tables in
> the database.

That's likely because there's no DML.


> We're at a point of diminishing returns here; I think any further
> improvement would require reducing the number of invalidation calls,
> as Andres was speculating about upthread.  Still, this shows that
> it doesn't take very much work to get a 10X improvement in the
> overhead associated with inval activities.  We've never seen this
> overhead stick out quite this much before, and maybe logical replication
> will always be an atypical workload, but I think this may be worth
> committing even if Andres does managed to cut the number of calls.

I've seen these callsites prominently in profiles not using logical
decoding.  Never quite as big as here, but still.  Temp table heavy
workloads IIRC are one of the easy way to trigger it.


> It would be interesting to see how much these patches help for your real
> use-case, as opposed to this toy example.  Assuming that the results are
> positive, I would advocate for back-patching these changes as far as 9.4
> where logical decoding came in.

+1.


> BTW, I also noticed that we're getting scarily close to exceeding
> MAX_SYSCACHE_CALLBACKS.  There are 30-some calls to
> CacheRegisterSyscacheCallback in our code, and while I think not all of
> them can be reached in a single process, we demonstrably get as high as 21
> registered callbacks in some regression test runs.  That's not leaving a
> lot of daylight for add-on modules.  The second patch attached includes
> increasing MAX_SYSCACHE_CALLBACKS from 32 to 64.  I think we'd be well
> advised to apply and back-patch that, even if we don't use the rest of
> the patch.

+1 - I'd advocate for doing so all the way.

Greetings,

Andres Freund


-- 
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general