On 6/24/24 17:14, Nathan Bossart wrote:
> On Mon, Jun 24, 2024 at 04:12:38PM +0200, Tomas Vondra wrote:
>> The important observation is that this only happens if a database is
>> created while the backup is running, and that it only happens with the
>> FILE_COPY strategy - I've never seen this with WAL_LOG (which is the
>> default since PG15).
> 
> My first thought is that this sounds related to the large comment in
> CreateDatabaseUsingFileCopy():
> 
>       /*
>        * We force a checkpoint before committing.  This effectively means that
>        * committed XLOG_DBASE_CREATE_FILE_COPY operations will never need to 
> be
>        * replayed (at least not in ordinary crash recovery; we still have to
>        * make the XLOG entry for the benefit of PITR operations). This avoids
>        * two nasty scenarios:
>        *
>        * #1: When PITR is off, we don't XLOG the contents of newly created
>        * indexes; therefore the drop-and-recreate-whole-directory behavior of
>        * DBASE_CREATE replay would lose such indexes.
>        *
>        * #2: Since we have to recopy the source database during DBASE_CREATE
>        * replay, we run the risk of copying changes in it that were committed
>        * after the original CREATE DATABASE command but before the system 
> crash
>        * that led to the replay.  This is at least unexpected and at worst 
> could
>        * lead to inconsistencies, eg duplicate table names.
>        *
>        * (Both of these were real bugs in releases 8.0 through 8.0.3.)
>        *
>        * In PITR replay, the first of these isn't an issue, and the second is
>        * only a risk if the CREATE DATABASE and subsequent template database
>        * change both occur while a base backup is being taken. There doesn't
>        * seem to be much we can do about that except document it as a
>        * limitation.
>        *
>        * See CreateDatabaseUsingWalLog() for a less cheesy CREATE DATABASE
>        * strategy that avoids these problems.
>        */
> 

Perhaps, the mentioned risks certainly seem like it might be related to
the issues I'm observing.

>> I don't recall any reports of similar issues from pre-15 releases, where
>> FILE_COPY was the only available option - I'm not sure why is that.
>> Either it didn't have this issue back then, or maybe people happen to
>> not create databases concurrently with a backup very often. It's a race
>> condition / timing issue, essentially.
> 
> If it requires concurrent activity on the template database, I wouldn't be
> surprised at all that this is rare.
> 

Right. Although, "concurrent" here means a somewhat different thing.
AFAIK there can't be a any changes concurrent with the CREATE DATABASE
directly, because we make sure there are no connections:

    createdb: error: database creation failed: ERROR:  source database
    "test" is being accessed by other users
    DETAIL:  There is 1 other session using the database.

But per the comment, it'd be a problem if there is activity after the
database gets copied, but before the backup completes (which is where
the replay will happen).

>> I see there have been a couple threads proposing various improvements to
>> FILE_COPY, that might make it more efficient/faster, namely using the
>> filesystem cloning [1] or switching pg_upgrade to use it [2]. But having
>> something that's (maybe) faster but not quite correct does not seem like
>> a winning strategy to me ...
>>
>> Alternatively, if we don't have clear desire to fix it, maybe the right
>> solution would be get rid of it?
> 
> It would be unfortunate if we couldn't use this for pg_upgrade, especially
> if it is unaffected by these problems.
> 

Yeah. I wouldn't mind using FILE_COPY in contexts where we know it's
safe, like pg_upgrade. I just don't want to let users to unknowingly
step on this.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Reply via email to