Re: [HACKERS] Assertion failure in syncrep.c
On 18 September 2014 07:32, Pavan Deolasee wrote: > 564 /* > 565 * Set state to complete; see SyncRepWaitForLSN() for discussion > of > 566 * the various states. > 567 */ > 568 thisproc->syncRepState = SYNC_REP_WAIT_COMPLETE; > 569 > 570 /* > 571 * Remove thisproc from queue. > 572 */ > 573 SHMQueueDelete(&(thisproc->syncRepLinks)); Yes, looks like a bugette to me also. Unlikely to occur except when no network involved, and even more luckily no effect at all when Assert is not enabled. So not a priority fix. But fix looks trivial, just to switch around the two statements above. I'll backpatch tomorrow. Thanks -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Assertion failure in syncrep.c
Pavan, * Pavan Deolasee (pavan.deola...@gmail.com) wrote: > On Mon, Sep 22, 2014 at 6:50 PM, Stephen Frost wrote: > > I agree that it looks like there may be a race condition there- but > > could you provide the test cases you're working with? What kind of > > behavior is it that's making it show up easily for you? > > > Nothing special really. Set up a 2-node sync replication on my Mac laptop > and running pgbench with 10 clients triggers it. As I said, after looking > at the code and realising that there is a race condition, I tried with with > gdb to reproduce the race I suspect. Well, that's a bit concerning. > Anyways, the attached patch should trigger the race condition for a simple > query. I'm deliberately making backend to wait to give walsender a chance > to send outstanding WALs and then making walsender to wait so that > assertion is triggered in the backend. Great, thanks, I'll try and look into this tomorrow. Stephen signature.asc Description: Digital signature
Re: [HACKERS] Assertion failure in syncrep.c
On Mon, Sep 22, 2014 at 6:50 PM, Stephen Frost wrote: > Pavan, > > * Pavan Deolasee (pavan.deola...@gmail.com) wrote: > > On Thu, Sep 18, 2014 at 12:02 PM, Pavan Deolasee < > pavan.deola...@gmail.com> > > wrote: > > > While running some tests on REL9_2_STABLE branch, I saw an assertion > > > failure in syncrep.c. The stack trace looks like this: > > > > > Any comments on this? I see it very regularly during my pgbench tests. > > I agree that it looks like there may be a race condition there- but > could you provide the test cases you're working with? What kind of > behavior is it that's making it show up easily for you? > > Nothing special really. Set up a 2-node sync replication on my Mac laptop and running pgbench with 10 clients triggers it. As I said, after looking at the code and realising that there is a race condition, I tried with with gdb to reproduce the race I suspect. Anyways, the attached patch should trigger the race condition for a simple query. I'm deliberately making backend to wait to give walsender a chance to send outstanding WALs and then making walsender to wait so that assertion is triggered in the backend. Hope this helps. Thanks, Pavan -- Pavan Deolasee http://www.linkedin.com/in/pavandeolasee syncrep_assertion_trigger.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Assertion failure in syncrep.c
Pavan, * Pavan Deolasee (pavan.deola...@gmail.com) wrote: > On Thu, Sep 18, 2014 at 12:02 PM, Pavan Deolasee > wrote: > > While running some tests on REL9_2_STABLE branch, I saw an assertion > > failure in syncrep.c. The stack trace looks like this: > > > Any comments on this? I see it very regularly during my pgbench tests. I agree that it looks like there may be a race condition there- but could you provide the test cases you're working with? What kind of behavior is it that's making it show up easily for you? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Assertion failure in syncrep.c
On Thu, Sep 18, 2014 at 12:02 PM, Pavan Deolasee wrote: > Hi All, > > While running some tests on REL9_2_STABLE branch, I saw an assertion > failure in syncrep.c. The stack trace looks like this: > > Any comments on this? I see it very regularly during my pgbench tests. Thanks, Pavan -- Pavan Deolasee http://www.linkedin.com/in/pavandeolasee
[HACKERS] Assertion failure in syncrep.c
Hi All,
While running some tests on REL9_2_STABLE branch, I saw an assertion
failure in syncrep.c. The stack trace looks like this:
frame #3: 0x0001055a2da9
postgres`ExceptionalCondition(conditionName=0x00010567b8c5,
errorType=0x0001055ff193, fileName=0x00010567b8f4, lineNumber=257)
+ 137 at assert.c:54
frame #4: 0x0001053f19d2
postgres`SyncRepWaitForLSN(XactCommitLSN=XLogRecPtr at 0x7fff5ab80d08)
+ 1410 at syncrep.c:257
frame #5: 0x0001050f9a52 postgres`RecordTransactionCommit + 1586 at
xact.c:1221
frame #6: 0x0001050f3e5f postgres`CommitTransaction + 303 at
xact.c:1920
frame #7: 0x0001050f38f4 postgres`CommitTransactionCommand + 180 at
xact.c:2588
frame #8: 0x00010543fd8e postgres`finish_xact_command + 126 at
postgres.c:2409
The failing assertion is this:
251 /*
252 * WalSender has checked our LSN and has removed us from queue.
Clean up
253 * state and leave. It's OK to reset these shared memory fields
without
254 * holding SyncRepLock, because any walsenders will ignore us
anyway when
255 * we're not on the queue.
256 */
257 Assert(SHMQueueIsDetached(&(MyProc->syncRepLinks)));
My theory is, and I did validate by using breakpoints in gdb, the code
in SyncRepWakeQueue() sets the proc->syncRepState to SYNC_REP_WAIT_COMPLETE.
564 /*
565 * Set state to complete; see SyncRepWaitForLSN() for
discussion of
566 * the various states.
567 */
568 thisproc->syncRepState = SYNC_REP_WAIT_COMPLETE;
569
570 /*
571 * Remove thisproc from queue.
572 */
573 SHMQueueDelete(&(thisproc->syncRepLinks));
But before walsender could detach the proc from the syncRepLinks shared
queue, the backend checks for the MyProc->syncRepState and decides not to
sleep after all and goes on to check the above Assert. The assertion fails
because the proc is not yet removed from the queue.
182 syncRepState = MyProc->syncRepState;
183 if (syncRepState == SYNC_REP_WAITING)
184 {
185 LWLockAcquire(SyncRepLock, LW_SHARED);
186 syncRepState = MyProc->syncRepState;
187 LWLockRelease(SyncRepLock);
188 }
189 if (syncRepState == SYNC_REP_WAIT_COMPLETE)
190 break;
191
We could just remove the assert, but then there is another similar assert
at the start of the function and it may hit for similar reasons (though
very unlikely). Instead, we could just reverse the order of setting
syncRepState and removing proc from syncRepLinks queue. There is a very
rare possibility that the proc is removed from the Q, but before its state
is changed, walsender crashes. I wonder if that cause infinite wait for the
process.
BTW, even though I was working on 9_2_STABLE branch, looks like issue
exists in the master branch too.
Thanks,
Pavan
--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee
