Re: [sqlsmith] Crash reading pg_stat_activity

From: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andreas Seltenreich <seltenreich(at)gmx(dot)de>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sqlsmith] Crash reading pg_stat_activity
Date: 2017-01-04 22:03:10
Message-ID: CAEepm=3WXeoOYNEesAHx8qorJq_PF-_3r9Vh35JLyKpoe9yZ_A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jan 5, 2017 at 10:23 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Tue, Dec 27, 2016 at 9:28 PM, Thomas Munro
> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>> On Wed, Dec 28, 2016 at 11:57 AM, Thomas Munro
>> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>>> But I'm starting to think that the best way might be to do BOTH of the
>>> things I said in my previous message: make dsa.c register on
>>> create/attach and also unregister before detaching iff the name was
>>> supplied at creation time for the benefit of extension writers, but
>>> make it not do anything at all about tranche name
>>> registration/unregistration if NULL was passed in at create time.
>>> Then register this particular tranche (LWTRANCHE_PARALLEL_QUERY_DSA)
>>> in every process in RegisterLWLockTranches. That way, you'd get a
>>> useful name in pg_stat_activity for other backends that are running
>>> parallel queries if they are ever waiting for these locks (unlikely
>>> but interesting to know abotu if it happens).
>>
>> Maybe something like the attached.
>
> Now that array_base and array_stride are gone, I don't see any reason
> why the DSA machinery needs to be aware of tranche names at all. So I
> propose to rip all that out, as in the attached.

The way I proposed makes it a lot easier to work with dynamic names so
you can differentiate variable numbers of areas; the names would have
exactly the right extent and they'd get unregistered in each backend
at just the right time. On the other hand, I don't really like seeing
lock tranche stuff leaking into other APIs like this, and using
tranche IDs in any way other than allocating a small number of them at
startup isn't really supported anyway, so +1 for doing it your way.

At one stage I had an idea that that argument was naming the DSA area,
not the lock tranche, and then the lock tranche happened to use a name
that was built from that name, but that doesn't make any sense if it's
optional depending on whether you already registered the lock
tranche...

- char lwlock_tranche_name[DSA_MAXLEN];

You can remove the now-unused DSA_MAXLEN macro.

--
Thomas Munro
http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2017-01-04 22:48:20 Re: Odd behavior with PG_TRY
Previous Message Tomas Vondra 2017-01-04 21:57:09 Re: multivariate statistics (v19)