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