Re: Issue with server side statement-level rollback

Hi,

Le 19/11/2020 à 21:43, Andres Freund a écrit :
> Hi,
>
> On 2020-11-12 11:40:22 +0100, Gilles Darold wrote:
>> The problem we are encountering is when PostgreSQL is compiled in debug
>> mode with --enable-cassert. At line 1327 of src/backend/tcop/pquery.c
>> the following assert fail:
>>
>>      /*
>>       * Clear subsidiary contexts to recover temporary memory.
>>       */
>>      Assert(portal->portalContext == CurrentMemoryContext);
>>
>>      MemoryContextDeleteChildren(portal->portalContext);
>>
>> This extension, although it is a risky implementation, works extremely
>> well when used in a fully controlled environment. It avoid the latency
>> of the extra communication for the RELEASE+SAVEPOINT usually controlled at
>> client side. The client is only responsible to issue the "ROLLBACK TO
>> autosavepoint"
>> when needed.  The extension allow a high performances gain for this feature
>> that helps customers using Oracle or DB2 to migrate to PostgreSQL.
>>
>>
>> Actually with the extension the memory context is not CurrentMemoryContext
>> as expected by the assert.
> What is it instead? I don't think you really can safely be in a
> different context at this point. There's risks of CurrentMemoryContext
> pointing to a deleted context, and risks of memory leaks, depending on
> the situation.

This is a PortalContext. Yes this implementation has some risks but 
until now I have not met any problem because its use and the environment 
are fully controlled.


>
>> So my question is should we allow such use through an extension and in
>> this case what is the change to PostgreSQL code that could avoid the
>> assert crash? Or perhaps we have missed something in this extension to
>> be able to make the assert happy but I don't think so.
> Without more detail of what you actually are precisely doing, and what
> the hooks / integration you'd like would look like, it's hard to comment
> usefully here.


We have implemented an extension to allow server side "statement-level 
rollback" with what is possible to do now with PG but the objective was 
to do the same thing that what was proposed as a core patch submitted by 
Takayuki Tsunakawa [1] . This patch will not be included into core and 
what I'm trying to do now is to have some facilities to allow this 
feature through an extension that does not suffer from the same 
limitation of pg_statement_rollback.


Looking that this patch for example, if we have a hook on 
finish_xact_command(), finish_xact_command() and 
AbortCurrentTransaction() I think we could probably be able to implement 
the feature through an extension in a more "safe" way. A hook on 
start_xact_command() seems useless as it looks it is executed before the 
UtilityProcess and Executor* hooks. See attached patch for an example of 
what could be useful for this kind of extension. Unfortunately my 
knowledge doesn't allow me to see further and especially if there is 
drawbacks. I hope this is more clear, I will work later on a POC to 
demonstrate the use case I want to implement.


[1] 
https://www.postgresql.org/message-id/flat/0A3221C70F24FB45833433255569204D1F6A9286%40G01JPEXMBYT05


-- 
Gilles Darold
http://www.darold.net/