Re: [PATCH] Hooks at XactCommand level

Le 31/07/2021 à 01:28, Andres Freund a écrit :
>
>>> 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