Re: [HACKERS] max_worker_processes on the standby

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Petr Jelinek <petr(at)2ndquadrant(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, oonishitk(at)nttdata(dot)co(dot)jp, pgsql-docs <pgsql-docs(at)postgresql(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] max_worker_processes on the standby
Date: 2015-10-20 19:05:22
Message-ID: 20151020190522.GT3391@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-docs pgsql-hackers

Robert Haas wrote:
> On Sat, Oct 17, 2015 at 5:37 PM, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:
> > I agree with that sentiment.
> >
> > Attached patch adds variable to the shmem which is used for module
> > activation tracking - set to true in ActiveCommitTs() and false in
> > DeactivateCommitTs(). All the checks inside the commit_ts code were changed
> > to use this new variable. I also removed the static variable Alvaro added in
> > previous commit because it's not needed anymore.
>
> That sounds good to me. On a quick read-through it looks OK too.

A revised version is attached. Two changes on top of Petr's patch:

1. In the two "get" routines, we were reading the flag without grabbing
the lock. This is okay in a master server, because the flag cannot
change in flight, but in a standby it is possible to have the module
be deactivated while TS data is being queried. To fix this, simply move
the check for the active shmem flag a few lines down to be inside the
locked section.

There are two other places that also read the flag without grabbing the
lock. These look okay to me, so I added comments stating so.

2. In TransactionIdGetCommitTsData() we were grabbing lock, reading some
data, releasing lock, then examining the "cached" value in shmem without
a lock to see if it matched the function argument; if it's match, grab
lock again and return the correct data. In the original coding this
made sense because there was no locked section prior to reading the
cache, but after the patch this was pointless. Make it simpler by
moving the read of the cache inside the locked section too.

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

Attachment Content-Type Size
committs-activation-fixes-2.patch text/x-diff 11.2 KB

In response to

Responses

Browse pgsql-docs by date

  From Date Subject
Next Message Rob Sargent 2015-10-21 23:34:58 ubuntu deb packaging
Previous Message Robert Haas 2015-10-20 13:12:24 Re: [HACKERS] max_worker_processes on the standby

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2015-10-20 19:11:53 Re: Multi-column distinctness.
Previous Message Andres Freund 2015-10-20 18:55:46 Re: pgbench throttling latency limit