Re: pg_receivewal fail to streams when the partial file to write is not fully initialized present in the wal receiver directory

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: mahendrakar s <mahendrakarforpg(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pg_receivewal fail to streams when the partial file to write is not fully initialized present in the wal receiver directory
Date: 2022-08-08 06:29:19
Message-ID: CALj2ACWHdpyX0fQPurhdCp91-Y+r9=g8yke_jZNMEaW0obaF=w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Aug 4, 2022 at 11:59 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Sun, Jul 31, 2022 at 8:36 PM mahendrakar s
> <mahendrakarforpg(at)gmail(dot)com> wrote:
> >
> >> On Mon, 25 Jul 2022 at 16:42, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >> Here's the v3 patch after rebasing.
> >>
> >> I just would like to reiterate the issue the patch is trying to solve:
> >> At times (when no space is left on the device or when the process is
> >> taken down forcibly (VM/container crash)), there can be leftover
> >> uninitialized .partial files (note that padding i.e. filling 16MB WAL
> >> files with all zeros is done in non-compression cases) due to which
> >> pg_receivewal fails to come up after the crash. To address this, the
> >> proposed patch makes the padding 16MB WAL files atomic (first write a
> >> .temp file, pad it and durably rename it to the original .partial
> >> file, ready to be filled with received WAL records). This approach is
> >> similar to what core postgres achieves atomicity while creating new
> >> WAL file (see how XLogFileInitInternal() creates xlogtemp.%d file
> >> first and then how InstallXLogFileSegment() durably renames it to
> >> original WAL file).
> >>
> >> Thoughts?
> >
> > Hi Bharath,
> >
> > Idea to atomically allocate WAL file by creating tmp file and renaming it is nice.
>
> Thanks for reviewing it.
>
> > I have one question though:
> > How is partially temp file created will be cleaned if the VM crashes or out of disk space cases? Does it endup creating multiple files for every VM crash/disk space during process of pg_receivewal?
> >
> > Thoughts?
>
> It is handled in the patch, see [1].
>
> Attaching v4 patch herewith which now uses the temporary file suffix
> '.tmp' as opposed to v3 patch '.temp'. This is just to be in sync with
> other atomic file write codes in the core - autoprewarm,
> pg_stat_statement, slot, basebacup, replorigin, snapbuild, receivelog
> and so on.
>
> Please review the v4 patch.

I've done some more testing today (hacked the code a bit by adding
pg_usleep(10000L); in pre-padding loop and crashing the pg_receivewal
process to produce the warning [1]) and found that the better place to
remove ".partial.tmp" leftover files is in FindStreamingStart()
because there we do a traversal of all the files in target directory
along the way to remove if ".partial.tmp" file(s) is/are found.

Please review the v5 patch further.

[1] pg_receivewal: warning: segment file
"0000000100000006000000B9.partial" has incorrect size 15884288,
skipping

--
Bharath Rupireddy
RDS Open Source Databases: https://aws.amazon.com/rds/postgresql/

Attachment Content-Type Size
v5-0001-Avoid-partially-padded-files-under-pg_receivewal-.patch application/octet-stream 6.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2022-08-08 06:40:53 Re: Logical replication - schema change not invalidating the relation cache
Previous Message Thomas Munro 2022-08-08 06:15:46 Re: logical decoding and replication of sequences