Re: Improve the performance of the standby server when dropping tables on the primary server

From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: "Tokuda, Takashi" <tokuda(dot)takashi(at)jp(dot)fujitsu(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Improve the performance of the standby server when dropping tables on the primary server
Date: 2017-08-01 06:55:37
Message-ID: CANP8+j+-m2XbB-YcexPz5NVE_FjV4rm3sav=WmxwGq_9FSw+uQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 1 August 2017 at 05:45, Tokuda, Takashi
<tokuda(dot)takashi(at)jp(dot)fujitsu(dot)com> wrote:
> Hi,
>
> The attached patch changes data structure storing unowned SMgrRelation objects
> from list structure to hash structure.
> The reason why I change it is that list structure very slowly removes a node.
> And list structure takes longer time to remove a node than hash structure.
>
> The problem was reported in BUG #14575.
> https://www.postgresql.org/message-id/20170303023246.25054.66379@wrigleys.postgresql.org
>
> In my customer's case, the standby server was delayed more than 20 minites
> when dropping many table at once.
>
>
> - Performance check

Interesting, thanks for the patch.

Couple of points regarding performance...

* The previous coding allowed for a fast path to access the last
unowned relation, which this patch removes. It seems a good idea to
cache the last unowned relation, or if not explain why the comment
that says why it worked that way is no longer true.

* We should only create the hash table when needed, i.e. on or after
when we add an unowned relation, since that is not a typical case.

* The hash table is sized at 400 elements and will grow from there.
The comments in dynahash say "An overly large nelem will penalize
hash_seq_search speed without buying much." so this makes your patch
suitable for the bulk case but likely to perform worse for fewer
elements. So I'm guessing that you picked 400 because that's what the
parameter is set to for the smgr relation table rather than because
this has had good consideration.

I'll take your word for now that it improves the main case but I'd
suggest you consider the performance effects of the patch on other
cases that use this code.

Without looking deeper: does this code also run for temp objects? Can
it be optimized for that case a little better?

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2017-08-01 07:37:36 Re: foreign table creation and NOT VALID check constraints
Previous Message Amit Langote 2017-08-01 06:34:00 Re: Update description of \d[S+] in \?