Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Andreas Seltenreich <seltenreich(at)gmx(dot)de>, pgsql-hackers(at)postgresql(dot)org, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Subject: Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM
Date: 2017-10-03 14:31:42
Message-ID: 10840.1507041102@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> Fixed the pstrdup problem by replacing with strlcpy() to stack-allocated
> variables (rather than palloc + memcpy as proposed in Michael's patch).

+1

> Tom Lane wrote:
>> I don't especially like the Asserts inside spinlocks, either.

> I didn't change these. It doesn't look to me that these asserts are
> worth very much as production code.

OK. If we ever see these hit in the buildfarm I might argue for
reconsidering, but without some evidence of that sort it's not
worth much concern.

>> I'm also rather befuddled by the fact that this code sets and clears
>> walrcv->latch outside the critical sections. If that field is used
>> by any other process, surely that's completely unsafe. If it isn't,
>> why is it being kept in shared memory?

> I think the latch is only used locally. Seems that it was only put in
> shmem to avoid a separate variable ...

Hm, I'm strongly tempted to move it to a separate static variable then.
That's not a bug fix, so maybe it only belongs in HEAD, but is there
value in keeping the branches in sync in this code? It sounded from
your commit message like they were pretty different already :-(

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2017-10-03 14:50:23 Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM
Previous Message Sokolov Yura 2017-10-03 14:30:03 Two pass CheckDeadlock in contentent case