Re: [BUGS] BUG #6218: TRAP: FailedAssertion( "!(owner->nsnapshots == 0)", File: "resowner.c", Line: 365)
Alvaro Herrera writes: > Excerpts from Tom Lane's message of lun sep 26 13:26:37 -0300 2011: >> The place where that refcount normally gets dropped is >> AtEarlyCommit_Snapshot, but that isn't going to be called at all in >> aborted-transaction cleanup. Worse, if we just transposed it over to be >> called in a place in AbortTransaction comparable to where it's called >> during commit, that still wouldn't fix the problem, because when the >> ROLLBACK happens, we've already aborted the transaction. > ... ouch. > Shall I work on a fix? I expect you are plenty busy with commitfest > stuff, but please let me know otherwise. I have what-I-think-is-the-fix pretty clear in my own mind, so let me give it a try. If it doesn't work I'll bounce it back to you. regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #6218: TRAP: FailedAssertion( "!(owner->nsnapshots == 0)", File: "resowner.c", Line: 365)
Excerpts from Tom Lane's message of lun sep 26 13:26:37 -0300 2011: > > y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) writes: > >> Maybe, but I'd still like to see a test case, because I can't reproduce > >> any such problem by preparing ROLLBACK in an aborted transaction. > > > reading GetTransactionSnapshot, it seems that the problem happens > > only with IsolationUsesXactSnapshot() true. > > Hmm. I'm inclined to think that this demonstrates a bug in snapshot > management, not so much in plancache. We have plancache doing > > PushActiveSnapshot(GetTransactionSnapshot()); > > and then later > > PopActiveSnapshot(); > > and at this point surely it is not plancache's fault if there is any > remaining refcount for the snapshot. There is, though, because > GetTransactionSnapshot saved a refcount in TopTransactionResourceOwner. > I think it's snapmgr.c's responsibility to make sure that that's cleaned > up, and it's not doing so. Agreed. > The place where that refcount normally gets dropped is > AtEarlyCommit_Snapshot, but that isn't going to be called at all in > aborted-transaction cleanup. Worse, if we just transposed it over to be > called in a place in AbortTransaction comparable to where it's called > during commit, that still wouldn't fix the problem, because when the > ROLLBACK happens, we've already aborted the transaction. ... ouch. > I think that AtEarlyCommit_Snapshot is misdesigned, and that far from > being done "early" in commit/abort, it needs to be done "late", like > somewhere not very long before the > ResourceOwnerDelete(TopTransactionResourceOwner) calls. There is no > very good reason to think that someone might not ask for a snapshot > during commit processing. > > Alvaro, do you happen to remember why this got designed as an "early" > transaction shutdown action, rather than delaying it as long as > possible? As far as I remember, the only principle was that it had to run before ResourceOwner cleanup. Commit 7b640b0345dc4fbd39ff568700985b432f6afa07 introduces that "early" call; ResOwner support had been introduced 10 days before in 6bbef4e5383c99d93aa974e2c79d328cfbd1c4a9. I probably just tried it out and noticed that resowner.c complained if I didn't drop the refcount prior to its own cleanup. I don't think I ever considered the scenario of calls in aborted transactions. Shall I work on a fix? I expect you are plenty busy with commitfest stuff, but please let me know otherwise. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #6218: TRAP: FailedAssertion( "!(owner->nsnapshots == 0)", File: "resowner.c", Line: 365)
y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) writes: >> Maybe, but I'd still like to see a test case, because I can't reproduce >> any such problem by preparing ROLLBACK in an aborted transaction. > reading GetTransactionSnapshot, it seems that the problem happens > only with IsolationUsesXactSnapshot() true. Hmm. I'm inclined to think that this demonstrates a bug in snapshot management, not so much in plancache. We have plancache doing PushActiveSnapshot(GetTransactionSnapshot()); and then later PopActiveSnapshot(); and at this point surely it is not plancache's fault if there is any remaining refcount for the snapshot. There is, though, because GetTransactionSnapshot saved a refcount in TopTransactionResourceOwner. I think it's snapmgr.c's responsibility to make sure that that's cleaned up, and it's not doing so. The place where that refcount normally gets dropped is AtEarlyCommit_Snapshot, but that isn't going to be called at all in aborted-transaction cleanup. Worse, if we just transposed it over to be called in a place in AbortTransaction comparable to where it's called during commit, that still wouldn't fix the problem, because when the ROLLBACK happens, we've already aborted the transaction. I think that AtEarlyCommit_Snapshot is misdesigned, and that far from being done "early" in commit/abort, it needs to be done "late", like somewhere not very long before the ResourceOwnerDelete(TopTransactionResourceOwner) calls. There is no very good reason to think that someone might not ask for a snapshot during commit processing. Alvaro, do you happen to remember why this got designed as an "early" transaction shutdown action, rather than delaying it as long as possible? regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #6218: TRAP: FailedAssertion( "!(owner->nsnapshots == 0)", File: "resowner.c", Line: 365)
hi, > y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) writes: >>> There isn't terribly much we can do with this report unless you can >>> provide a complete test case to reproduce it. > >> after some investigation, i think it is triggered by protocol-level prepare >> of ROLLBACK in an aborted transaction. does the following patch make sense? > > Maybe, but I'd still like to see a test case, because I can't reproduce > any such problem by preparing ROLLBACK in an aborted transaction. reading GetTransactionSnapshot, it seems that the problem happens only with IsolationUsesXactSnapshot() true. the attached program triggers the assertion failure reliably for me. YAMAMOTO Takashi > > regards, tom lane #include #include #include #include int main() { PGconn *conn; PGresult *res; conn = PQconnectdb(""); assert(PQstatus(conn) == CONNECTION_OK); res = PQexec(conn, "SET SESSION CHARACTERISTICS AS " "TRANSACTION ISOLATION LEVEL REPEATABLE READ"); assert(PQresultStatus(res) == PGRES_COMMAND_OK); PQclear(res); res = PQexec(conn, "BEGIN"); assert(PQresultStatus(res) == PGRES_COMMAND_OK); PQclear(res); res = PQexec(conn, "hoge"); /* intentional syntax error */ assert(PQresultStatus(res) != PGRES_COMMAND_OK); PQclear(res); assert(PQtransactionStatus(conn) == PQTRANS_INERROR); res = PQprepare(conn, "r", "ROLLBACK", 0, NULL); assert(PQresultStatus(res) == PGRES_COMMAND_OK); PQclear(res); res = PQexecPrepared(conn, "r", 0, NULL, NULL, NULL, 0); assert(PQresultStatus(res) == PGRES_COMMAND_OK); PQclear(res); exit(0); } -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #6218: TRAP: FailedAssertion( "!(owner->nsnapshots == 0)", File: "resowner.c", Line: 365)
y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) writes: >> There isn't terribly much we can do with this report unless you can >> provide a complete test case to reproduce it. > after some investigation, i think it is triggered by protocol-level prepare > of ROLLBACK in an aborted transaction. does the following patch make sense? Maybe, but I'd still like to see a test case, because I can't reproduce any such problem by preparing ROLLBACK in an aborted transaction. regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #6218: TRAP: FailedAssertion( "!(owner->nsnapshots == 0)", File: "resowner.c", Line: 365)
hi, > "YAMAMOTO Takashi" writes: >> 9.2devel (2562dcea811eb642e1c5442e1ede9fe268278157) > >> ERROR: could not serialize access due to concurrent update >> STATEMENT: UPDATE file SET atime = current_timestamp WHERE fileid = $1 >> TRAP: FailedAssertion("!(owner->nsnapshots == 0)", File: "resowner.c", Line: >> 365) >> LOG: server process (PID 16832) was terminated by signal 6: Abort trap > > There isn't terribly much we can do with this report unless you can > provide a complete test case to reproduce it. > > regards, tom lane after some investigation, i think it is triggered by protocol-level prepare of ROLLBACK in an aborted transaction. does the following patch make sense? YAMAMOTO Takashi diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index 987..d1da7df 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -520,6 +520,7 @@ RevalidateCachedQuery(CachedPlanSource *plansource) * don't have invalidatable plans, so we'd not get here for such a * command. */ + Assert(analyze_requires_snapshot(plansource->raw_parse_tree)); snapshot_set = false; if (!ActiveSnapshotSet()) { @@ -757,13 +758,11 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist, /* * If a snapshot is already set (the normal case), we can just use -* that for parsing/planning. But if it isn't, install one. Note: no -* point in checking whether parse analysis requires a snapshot; -* utility commands don't have invalidatable plans, so we'd not get -* here for such a command. +* that for parsing/planning. But if it isn't, install one. */ snapshot_set = false; - if (!ActiveSnapshotSet()) + if (!ActiveSnapshotSet() && + analyze_requires_snapshot(plansource->raw_parse_tree)) { PushActiveSnapshot(GetTransactionSnapshot()); snapshot_set = true; -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #6218: TRAP: FailedAssertion("!(owner->nsnapshots == 0)", File: "resowner.c", Line: 365)
"YAMAMOTO Takashi" writes: > 9.2devel (2562dcea811eb642e1c5442e1ede9fe268278157) > ERROR: could not serialize access due to concurrent update > STATEMENT: UPDATE file SET atime = current_timestamp WHERE fileid = $1 > TRAP: FailedAssertion("!(owner->nsnapshots == 0)", File: "resowner.c", Line: > 365) > LOG: server process (PID 16832) was terminated by signal 6: Abort trap There isn't terribly much we can do with this report unless you can provide a complete test case to reproduce it. regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs