Re: PATCH: Exclude unlogged tables from base backups

From: David Steele <david(at)pgmasters(dot)net>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: Exclude unlogged tables from base backups
Date: 2018-01-25 19:58:07
Message-ID: d4fc8171-ea01-161c-279d-4df48d270326@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 1/25/18 12:31 AM, Masahiko Sawada wrote:
> On Thu, Jan 25, 2018 at 3:25 AM, David Steele <david(at)pgmasters(dot)net> wrote:
>>>
>>> Here is the first review comments.
>>>
>>> + unloggedDelim = strrchr(path, '/');
>>>
>>> I think it doesn't work fine on windows. How about using
>>> last_dir_separator() instead?
>>
>> I think this function is OK on Windows -- we use it quite a bit.
>> However, last_dir_separator() is clearer so I have changed it.
>
> Thank you for updating this. I was concerned about a separator
> character '/' might not work fine on windows.

Ah yes, I see what you mean now.

> On Thu, Jan 25, 2018 at 6:23 AM, David Steele <david(at)pgmasters(dot)net> wrote:
>> On 1/24/18 4:02 PM, Adam Brightwell wrote:
>> Actually, I was talking to Stephen about this it seems like #3 would be
>> more practical if we just stat'd the init fork for each relation file
>> found. I doubt the stat would add a lot of overhead and we can track
>> each unlogged relation in a hash table to reduce overhead even more.
>>
>
> Can the readdir handle files that are added during the loop? I think
> that we still cannot exclude a new unlogged relation if the relation
> is added after we execute readdir first time. To completely eliminate
> it we need a sort of lock that prevents to create new unlogged
> relation from current backends. Or we need to do readdir loop multiple
> times to see if no new relations were added during sending files.

As far as I know readdir() is platform-dependent in terms of how it
scans the dir and if files created after the opendir() will appear.

It shouldn't matter, though, since WAL replay will recreate those files.

> If you're updating the patch to implement #3, this patch should be
> marked as "Waiting on Author". After updated I'll review it again.
Attached is a new patch that uses stat() to determine if the init fork
for a relation file exists. I decided not to build a hash table as it
could use considerable memory and I didn't think it would be much faster
than a simple stat() call.

The reinit.c refactor has been removed since it was no longer needed.
I'll submit the tests I wrote for reinit.c as a separate patch for the
next CF.

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

Attachment Content-Type Size
exclude-unlogged-v3.patch text/plain 8.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeremy Schneider 2018-01-25 20:03:36 Logical decoding of TRUNCATE vs DELETE
Previous Message Alvaro Herrera 2018-01-25 19:24:21 Re: [HACKERS] Proposal: Local indexes for partitioned table