PATCH: Exclude temp relations from base backup

Lists: pgsql-hackers
From: David Steele <david(at)pgmasters(dot)net>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: PATCH: Exclude temp relations from base backup
Date: 2018-02-28 15:55:10
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

This is a follow-up patch from the exclude unlogged relations discussion
[1].

The patch excludes temporary relations during a base backup using the
existing looks_like_temp_rel_name() function for identification.

It shares code to identify database directories from [1], so for now
that has been duplicated in this patch to make it independent. I'll
rebase depending on what gets committed first.

Thanks,
--
-David
david(at)pgmasters(dot)net

[1]
https://www.postgresql.org/message-id/04791bab-cb04-ba43-e9c0-664a4c1ffb2c%40pgmasters.net

Attachment Content-Type Size
backup-exclude-temp-rel-v1.patch text/plain 7.2 KB

From: David Steele <david(at)pgmasters(dot)net>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: PATCH: Exclude temp relations from base backup
Date: 2018-03-13 16:34:16
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

Hi,

On 2/28/18 10:55 AM, David Steele wrote:
> This is a follow-up patch from the exclude unlogged relations discussion
> [1].
>
> The patch excludes temporary relations during a base backup using the
> existing looks_like_temp_rel_name() function for identification.
>
> It shares code to identify database directories from [1], so for now
> that has been duplicated in this patch to make it independent. I'll
> rebase depending on what gets committed first.

Updated the patch to change die() to BAIL_OUT() and use append_to_file()
as suggested for another test patch [1].

Regards,
--
-David
david(at)pgmasters(dot)net

[1]
https://www.postgresql.org/message-id/6bc5d931-5b00-279f-f65a-26e32de400a6%40pgmasters.net

Attachment Content-Type Size
backup-exclude-temp-rel-v2.patch text/plain 8.0 KB

From: David Steele <david(at)pgmasters(dot)net>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: PATCH: Exclude temp relations from base backup
Date: 2018-03-23 16:54:44
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

On 3/13/18 12:34 PM, David Steele wrote:

> Updated the patch to change die() to BAIL_OUT() and use append_to_file()
> as suggested for another test patch.

Updated patch now that the unlogged table exclusions have been committed
[1].

Thanks,
--
-David
david(at)pgmasters(dot)net

[1]
https://www.postgresql.org/message-id/4d9be1c0-5c58-d9a0-7152-2771224910ae%40sigaev.ru

Attachment Content-Type Size
backup-exclude-temp-rel-v3.patch text/plain 6.4 KB

From: Teodor Sigaev <teodor(at)sigaev(dot)ru>
To: David Steele <david(at)pgmasters(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: PATCH: Exclude temp relations from base backup
Date: 2018-03-26 15:52:11
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

Hi!

Will autovacuum (or something else) complain about absense of relfile during
orphan table deleting? I mean, you get a base backup without temp tables, then
you try to run postgres on it and will it complain about existing record in
pg_class and absence of corresponding relfile?

David Steele wrote:
> On 3/13/18 12:34 PM, David Steele wrote:
>
>> Updated the patch to change die() to BAIL_OUT() and use append_to_file()
>> as suggested for another test patch.
>
> Updated patch now that the unlogged table exclusions have been committed
> [1].
>
> Thanks,
>

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/


From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Teodor Sigaev <teodor(at)sigaev(dot)ru>
Cc: David Steele <david(at)pgmasters(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: PATCH: Exclude temp relations from base backup
Date: 2018-03-26 17:06:19
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

Greetings,

* Teodor Sigaev (teodor(at)sigaev(dot)ru) wrote:
> Will autovacuum (or something else) complain about absense of relfile during
> orphan table deleting? I mean, you get a base backup without temp tables,
> then you try to run postgres on it and will it complain about existing
> record in pg_class and absence of corresponding relfile?

I would certainly hope not considering that's what happens during
regular crash recovery also, so if there's an issue with that, we'd have
a problem in released versions.

There's an independent discussion that was being had recently about how
to make sure those records in pg_class get cleaned up in a reasonable
timeframe and don't lead to problems with wrap-arounds, but that's a
different and pre-existing issue.

Thanks!

Stephen


From: David Steele <david(at)pgmasters(dot)net>
To: Stephen Frost <sfrost(at)snowman(dot)net>, Teodor Sigaev <teodor(at)sigaev(dot)ru>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: PATCH: Exclude temp relations from base backup
Date: 2018-03-26 17:08:50
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

On 3/26/18 1:06 PM, Stephen Frost wrote:
>
> * Teodor Sigaev (teodor(at)sigaev(dot)ru) wrote:
>> Will autovacuum (or something else) complain about absense of relfile during
>> orphan table deleting? I mean, you get a base backup without temp tables,
>> then you try to run postgres on it and will it complain about existing
>> record in pg_class and absence of corresponding relfile?
>
> I would certainly hope not considering that's what happens during
> regular crash recovery also, so if there's an issue with that, we'd have
> a problem in released versions.

Agreed. The logic for pg_basebackup was modeled off RemovePgTempFiles()
which is called at postmaster start. We are just doing the cleanup in
advance (in the backup only, of course).

Thanks,
--
-David
david(at)pgmasters(dot)net


From: Teodor Sigaev <teodor(at)sigaev(dot)ru>
To: David Steele <david(at)pgmasters(dot)net>, Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: PATCH: Exclude temp relations from base backup
Date: 2018-03-27 13:34:24
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

Thank you, pushed

David Steele wrote:
> On 3/26/18 1:06 PM, Stephen Frost wrote:
>>
>> * Teodor Sigaev (teodor(at)sigaev(dot)ru) wrote:
>>> Will autovacuum (or something else) complain about absense of relfile during
>>> orphan table deleting? I mean, you get a base backup without temp tables,
>>> then you try to run postgres on it and will it complain about existing
>>> record in pg_class and absence of corresponding relfile?
>>
>> I would certainly hope not considering that's what happens during
>> regular crash recovery also, so if there's an issue with that, we'd have
>> a problem in released versions.
>
> Agreed. The logic for pg_basebackup was modeled off RemovePgTempFiles()
> which is called at postmaster start. We are just doing the cleanup in
> advance (in the backup only, of course).
>
> Thanks,
>

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/