Re: [PATCH] Hooks at XactCommand level

I'm *very* unconvinced it makes sense to implement a feature like this
in an extension / that we should expose API for that purpose. To me the
top-level transaction state is way too tied to our internals for it to
be reasonably dealt with in an extension.

Yeah, that's the other major problem --- the use-case doesn't seem
very convincing.  I'm not even sold on the goal, let alone on trying
to implement it by hooking into these particular places.  I think
that'll end up being buggy and fragile as well as not very performant.

I'm more favorable than you on the overall goal. Migrations to PG are a
frequent and good thing and as discussed before, lots of PG forks ended
up implementing a version of this. Clearly there's demand.

Sorry for the response delay. I have though about adding this odd hook to be able to implement this feature through an extension because I don't think this is something that should be implemented in core. There were also patches proposals which were all rejected.

We usually implement the feature at client side which is imo enough for the use cases. But the problem is that this a catastrophe in term of performances. I have done a small benchmark to illustrate the problem. This is a single process client on the same host than the PG backend.

For 10,000 tuples inserted with 50% of failures and rollback at statement level handled at client side:

        Expected: 5001, Count:  5001
        DML insert took: 13 wallclock secs ( 0.53 usr +  0.94 sys =  1.47 CPU)

Now with statement at rollback level handled at server side using the hook and the extension:

        Expected: 5001, Count:  5001
        DML insert took:  4 wallclock secs ( 0.27 usr +  0.32 sys =  0.59 CPU)

And with 100,000 tuples this is worst. Without the extension:

        Expected: 50001, Count:  50001
        DML insert took: 1796 wallclock secs (14.95 usr + 20.29 sys = 35.24 CPU)

with server side Rollback at statement level:

        Expected: 50001, Count:  50001
        DML insert took: 372 wallclock secs ( 4.85 usr +  5.45 sys = 10.30 CPU)

I think this is not so uncommon use cases and that could shows the interest of such extension.

However, I think a proper implementation would require a substantial
amount of effort. Including things like optimizing the subtransaction
logic so that switching the feature on doesn't lead to xid wraparound
issues. Adding odd hooks doesn't move us towards a real solution imo.

I would like to help on this part but unfortunately I have no idea on how we can improve that.

Best regards,

-- 
Gilles Darold