Обсуждение: Assertion failure in syncrep.c

Поиск
Список
Период
Сортировка

Assertion failure in syncrep.c

От
Pavan Deolasee
Дата:
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: 0x00000001055a2da9 postgres`ExceptionalCondition(conditionName=0x000000010567b8c5, errorType=0x00000001055ff193, fileName=0x000000010567b8f4, lineNumber=257) + 137 at assert.c:54
    frame #4: 0x00000001053f19d2 postgres`SyncRepWaitForLSN(XactCommitLSN=XLogRecPtr at 0x00007fff5ab80d08) + 1410 at syncrep.c:257
    frame #5: 0x00000001050f9a52 postgres`RecordTransactionCommit + 1586 at xact.c:1221
    frame #6: 0x00000001050f3e5f postgres`CommitTransaction + 303 at xact.c:1920
    frame #7: 0x00000001050f38f4 postgres`CommitTransactionCommand + 180 at xact.c:2588
    frame #8: 0x000000010543fd8e postgres`finish_xact_command + 126 at postgres.c:2409
<snip>

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

Re: Assertion failure in syncrep.c

От
Pavan Deolasee
Дата:
On Thu, Sep 18, 2014 at 12:02 PM, Pavan Deolasee <pavan.deolasee@gmail.com> 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

Re: Assertion failure in syncrep.c

От
Stephen Frost
Дата:
Pavan,

* Pavan Deolasee (pavan.deolasee@gmail.com) wrote:
> On Thu, Sep 18, 2014 at 12:02 PM, Pavan Deolasee <pavan.deolasee@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?
Thanks!
    Stephen

Re: Assertion failure in syncrep.c

От
Pavan Deolasee
Дата:
On Mon, Sep 22, 2014 at 6:50 PM, Stephen Frost <sfrost@snowman.net> wrote:
Pavan,

* Pavan Deolasee (pavan.deolasee@gmail.com) wrote:
> On Thu, Sep 18, 2014 at 12:02 PM, Pavan Deolasee <pavan.deolasee@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
Вложения

Re: Assertion failure in syncrep.c

От
Stephen Frost
Дата:
Pavan,

* Pavan Deolasee (pavan.deolasee@gmail.com) wrote:
> On Mon, Sep 22, 2014 at 6:50 PM, Stephen Frost <sfrost@snowman.net> 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

Re: Assertion failure in syncrep.c

От
Simon Riggs
Дата:
On 18 September 2014 07:32, Pavan Deolasee <pavan.deolasee@gmail.com> 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