Re: Statement-level rollback

Hi,

On December 7, 2018 11:44:17 AM PST, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>On 2018-Dec-07, Andres Freund wrote:
>
>> Hi,
>>
>> On 2018-12-07 16:20:06 -0300, Alvaro Herrera wrote:
>> >          case TBLOCK_BEGIN:
>> > +            s->rollbackScope = XactRollbackScope;
>> >              s->blockState = TBLOCK_INPROGRESS;
>> > +            if (s->rollbackScope == XACT_ROLLBACK_SCOPE_STMT)
>> > +            {
>> > +                PushTransaction();
>> > +                s = CurrentTransactionState;    /* changed by push */
>> > +                s->name = MemoryContextStrdup(TopTransactionContext, "pg
>internal");
>> > +                StartSubTransaction();
>> > +                s->blockState = TBLOCK_SUBINPROGRESS;
>> > +            }
>>
>> Isn't this going to be performing ridiculously bad, to the point of
>> being not much but a trap for users?
>
>If you bulk-load with INSERT under this behavior, yeah it'll create
>lots
>of subtransactions, with the obvious downsides -- eating lots of xids
>for one.  I think the answer to that is "don't do that".  However, if
>you want to do 100k inserts and have the 10 bogus of those fail cleanly
>(and not abort the other 99990), that's clearly this patch's intention
>> I can see the feature being useful, but I don't think we should
>accept a
>> feature that'll turn bulkloading with insert into a server shutdown
>> scenario.
>
>Hm.

Isn't the realistic scenario for migrations that people will turn this feature on on a more global basis? If they need
todo per transaction choices, that makes this much less useful for easy migrations. 

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.