Re: Statement-level rollback

Hi,

On December 7, 2018 11:44:17 AM PST, Alvaro Herrera <alvherre(at)2ndquadrant(dot)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 to do 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.