Re: [sqlsmith] Crash reading pg_stat_activity

On Wed, Jan 4, 2017 at 5:03 PM, Thomas Munro
<thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> 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.

Only from the perspective of the backend that's using DSA. From the
perspective of some other backend reading pg_stat_activity, it's all
wrong. There, you want the name to registered as early as possible -
either at system startup time for builtin things or at module load
time for extensions. With the way you have it, you'd only be able to
recognize a lock wait as being related to parallel_query_dsa if your
session had previously executed a parallel query. That's clearly not
desirable. With this approach, _PG_init can do LWLockRegisterTranche
and then if you stick the library into shared_preload_libraries or
session_preload_libraries *all* backends have that tranche whether
they use the library or not. If the tranche registry were shared,
then your approach would be fine, but it isn't.

> 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.

OK.

> 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.

Ah, thanks. Committed with that change.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company