Thread: backend crash with FATAL: BeginInternalSubTransaction: unexpected state END
backend crash with FATAL: BeginInternalSubTransaction: unexpected state END
From
Frank van Vugt
Date:
Hi, Migrating a number of sql-functions to plpgsql-functions with added functionality resulted in a backend crash. # select version(); version ------------------------------------------------------------------------ PostgreSQL 8.2.4 on i686-pc-linux-gnu, compiled by GCC gcc (GCC) 3.4.3 (1 row) The problem is easily reproduced by a copy&paste of the following code in a terminal: ******************************************************** create table f1(id int); CREATE OR REPLACE FUNCTION f1_crash() RETURNS int LANGUAGE 'plpgsql' IMMUTABLE STRICT SECURITY INVOKER AS ' DECLARE result INT := 0; BEGIN BEGIN SELECT INTO STRICT result 1; EXCEPTION WHEN NO_DATA_FOUND THEN RAISE EXCEPTION ''Unknown record...!!''; WHEN TOO_MANY_ROWS THEN RAISE EXCEPTION ''More than one record found...''; END; RETURN result; END;'; CREATE OR REPLACE FUNCTION tr_f1_def() RETURNS trigger LANGUAGE 'plpgsql' VOLATILE STRICT SECURITY INVOKER AS ' DECLARE BEGIN IF f1_crash() THEN RAISE NOTICE ''We got to here...''; END IF; RETURN NULL; END;'; CREATE CONSTRAINT TRIGGER f1_def AFTER INSERT ON f1 DEFERRABLE INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE tr_f1_def(); ******************************************************** After which these statements will run ok: insert into f1 values (1); insert into f1 select * from generate_series(1, 1000); However this will fail: begin; insert into f1 select * from generate_series(1, 1000); commit; Resulting in: FATAL: BeginInternalSubTransaction: unexpected state END CONTEXT: PL/pgSQL function "f1_crash" line 4 at block variables initialization PL/pgSQL function "tr_f1_def" line 3 at if server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Succeeded. The problem seems to be caused by the nested BEGIN/END block in f1_crash(), but we need that there in order to separate the RETURN from the EXCEPTION block.... Cleanup: ******************************************************** drop table f1 cascade; drop function tr_f1_def(); drop function f1_crash(); ******************************************************** Looking forward to your remarks ! -- Best, Frank.
Frank van Vugt <ftm.van.vugt@foxi.nl> writes: > FATAL: BeginInternalSubTransaction: unexpected state END Hmm, do you get the impression that user-written constraint triggers aren't very well tested ;-) ? It looks to me like BeginInternalSubTransaction simply needs to allow TBLOCK_END (and TBLOCK_PREPARE too) as acceptable initial states, because these could be seen by a function executed during COMMIT or PREPARE TRANSACTION-time processing of deferred triggers. I think that the other states it rejects are OK, because we don't try to execute any user-written code while transiting through those TBLOCK states. regards, tom lane
Re: backend crash with FATAL: BeginInternalSubTransaction: unexpected state END
From
Frank van Vugt
Date:
> Hmm, do you get the impression that user-written constraint triggers > aren't very well tested ;-) ? Well, we users are here to serve ;) > It looks to me like BeginInternalSubTransaction simply needs to allow > TBLOCK_END (and TBLOCK_PREPARE too) as acceptable initial states Ok, so for patch-sake, when I change BeginInternalSubTransaction() in xact.c by moving these two cases to the upper set (TBLOCK_STARTED/INPROGRESS/SUBINPROGRESS), then I should be ok? At the moment, this looks like a showstopper, so I'd prefer patching and move on ;) -- Best, Frank.
Frank van Vugt <ftm.van.vugt@foxi.nl> writes: > Ok, so for patch-sake, when I change BeginInternalSubTransaction() in xact.c > by moving these two cases to the upper set > (TBLOCK_STARTED/INPROGRESS/SUBINPROGRESS), then I should be ok? Try it and see ... regards, tom lane
Re: backend crash with FATAL: BeginInternalSubTransaction: unexpected state END
From
Frank van Vugt
Date:
> > Ok, so for patch-sake, when I change BeginInternalSubTransaction() in > > xact.c by moving these two cases to the upper set > > (TBLOCK_STARTED/INPROGRESS/SUBINPROGRESS), then I should be ok? > Try it and see ... Been there, done that, seems to work (both the example as well as my usecase). Will keep an eye on the committers list to see what eventually makes it 'til there. What did the trick for me was: # diff -u src/backend/access/transam/xact.c_orig src/backend/access/transam/xact.c --- src/backend/access/transam/xact.c_orig 2007-05-30 17:38:10.000000000 +0200 +++ src/backend/access/transam/xact.c 2007-05-30 18:34:40.000000000 +0200 @@ -3319,6 +3319,8 @@ case TBLOCK_STARTED: case TBLOCK_INPROGRESS: case TBLOCK_SUBINPROGRESS: + case TBLOCK_END: + case TBLOCK_PREPARE: /* Normal subtransaction start */ PushTransaction(); s = CurrentTransactionState; /* changed by push */ @@ -3335,7 +3337,6 @@ case TBLOCK_DEFAULT: case TBLOCK_BEGIN: case TBLOCK_SUBBEGIN: - case TBLOCK_END: case TBLOCK_SUBEND: case TBLOCK_ABORT: case TBLOCK_SUBABORT: @@ -3345,7 +3346,6 @@ case TBLOCK_SUBABORT_PENDING: case TBLOCK_SUBRESTART: case TBLOCK_SUBABORT_RESTART: - case TBLOCK_PREPARE: elog(FATAL, "BeginInternalSubTransaction: unexpected state %s", BlockStateAsString(s->blockState)); break; So Tom, thanks a lot for your swift reply and solution ! -- Best, Frank.
Frank van Vugt <ftm.van.vugt@foxi.nl> writes: >>> Ok, so for patch-sake, when I change BeginInternalSubTransaction() in >>> xact.c by moving these two cases to the upper set >>> (TBLOCK_STARTED/INPROGRESS/SUBINPROGRESS), then I should be ok? >> Try it and see ... > Been there, done that, seems to work (both the example as well as my usecase). Great, thanks for testing. Will push the fix into CVS. regards, tom lane