Re: [BUGS] BUG #6218: TRAP: FailedAssertion( "!(owner->nsnapshots == 0)", File: "resowner.c", Line: 365)

Excerpts from Tom Lane's message of lun sep 26 20:59:45 -0300 2011:

> Well, I soon ran into the issue that delaying the snapshot release makes
> TopTransactionResourceOwner spit up. After some reflection I decided
> that the real problem is a circular dependency: snapshot management must
> be considered lower-level than ResourceOwners because ResourceOwners
> tell snapshot management what to do, but here we have
> GetTransactionSnapshot trying to use TopTransactionResourceOwner to
> manage its internal reference to the transaction snapshot.

Great. I noticed the circularity but didn't reflect that it was bogus
in itself.

> Accordingly, the attached proposed patch gets rid of the circularity
> by removing snapmgr.c's dependency on TopTransactionResourceOwner,
> in favor of having it track the refcount "manually". This was messier
> than I'd hoped because the bogus design had propagated into the SSI
> manager meanwhile, but removing the TopTransactionResourceOwner
> dependency from that too seems like a good idea.

To be honest, I panicked for a second when I saw the new
SnapshotResetXmin call, before I realized that it wasn't necessary
before. The serializable case makes more sense the patched way, I
think.

> This passes the regular and isolation regression tests, and it's also
> okay with Yamamoto-san's prepared-ROLLBACK test case even without the
> band-aid fix in plancache.c. I can't immediately think of any other
> test cases to throw at it.

I added tests for all the problematic cases discovered after snapmgr was
introduced, so at least most known weird spots are covered.

Thanks

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support