Re: Error on failed COMMIT

From: Dave Cramer <davecramer(at)postgres(dot)rocks>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "Haumacher, Bernhard" <haui(at)haumacher(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Error on failed COMMIT
Date: 2020-02-14 18:04:07
Message-ID: CADK3HHJeVbUv4gRDWP5D6m3DTRTOPbj-L1+N9XVUnn2hvMbO8w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 14 Feb 2020 at 12:37, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Thu, Feb 13, 2020 at 2:38 AM Haumacher, Bernhard <haui(at)haumacher(dot)de>
> wrote:
> > Assume the application is written in Java and sees Postgres through the
> > JDBC driver:
> >
> > composeTransaction() {
> > Connection con = getConnection(); // implicitly "begin"
> > try {
> > insertFrameworkLevelState(con);
> > insertApplicationLevelState(con);
> > con.commit();
> > publishNewState();
> > } catch (Throwable ex) {
> > con.rollback();
> > }
> > }
> >
> > With the current implementation, it is possible, that the control flow
> > reaches "publishNewState()" without the changes done in
> > "insertFrameworkLevelState()" have been made persistent - without the
> > framework-level code (which is everything except
> > "insertApplicationLevelState()") being able to detect the problem (e.g.
> > if "insertApplicationLevelState()" tries add a null into a non-null
> > column catching the exception or any other application-level error that
> > is not properly handled through safepoints).
> >
> > From a framework's perspective, this behavior is absolutely
> > unacceptable. Here, the framework-level code sees a database that
> > commits successfully but does not make its changes persistent.
> > Therefore, I don't think that altering this behavior has more downside
> > than upside.
>
> I am not sure that this example really proves anything. If
> insertFrameworkLevelState(con), insertApplicationLevelState(con), and
> con.commit() throw exceptions, or if they return a status code and you
> check it and throw an exception if it's not what you expect, then it
> will work.

Thing is that con.commit() DOESN'T return a status code, nor does it throw
an exception as we silently ROLLBACK here.

As noted up thread it's somewhat worse as depending on how the transaction
failed we seem to do different things

In Tom's example we do issue a warning and say there is no transaction
running. I would guess we silently rolled back earlier.
In my example we don't issue the warning we just roll back.

I do agree with Tom that changing this behaviour at this point would make
things worse for more people than it would help so I am not advocating
throwing an error here.

I would however advocate for consistently doing the same thing with failed
transactions

Dave Cramer

www.postgres.rocks

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2020-02-14 18:07:34 Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager
Previous Message Tom Lane 2020-02-14 17:39:19 Re: Marking some contrib modules as trusted extensions