Date: Mon, 18 Oct 2021 11:40:35 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Pg Bugs <pgsql-bugs@postgresql.org>,
	Andrew B <pgsql.20.drkshadow@spamgourmet.com>
Subject: Re: Snapshot leak warning with lo_export in subtransaction
Message-ID: <202110181440.fdiy6yrecsuk@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <f698e989-df50-9e4e-d7c3-3c05ebe56ddd@iki.fi>
Archived-At: 
 <https://www.postgresql.org/message-id/202110181440.fdiy6yrecsuk%40alvherre.pgsql>
Precedence: bulk

On 2021-Oct-18, Heikki Linnakangas wrote:

> I tried that, but didn't like the result. It conflated the user-visible
> INV_READ/WRITE flags with the new internal-only flag.

True -- I also noticed this and had written in my earlier reply that we
could perhaps define this flag internally, not exposing it to the user.
But that seemed weird from the API definition perspective, so I removed
that phrase before sending.

> Thinking about this some more, I came up with the attached. It moves the
> responsibility of registering the snapshot from inv_api.c to the caller.

I like this patch much better.

> With that change, there's no need for a new option to inv_open(). The
> division of labor between be-fsstubs.c and inv_api.c has always been a bit
> blurry, I think that this makes it slightly more clear.

Agreed.

-- 
Álvaro Herrera              Valdivia, Chile  —  https://www.EnterpriseDB.com/