On Sun, Jul 31, 2022 at 8:36 PM mahendrakar s
<mahendrakarforpg@gmail.com> wrote:
>
>> On Mon, 25 Jul 2022 at 16:42, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.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
creatingmultiple 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.
[1]
+ /*
+ * Actual file doesn't exist. Now, create a temporary file pad it
+ * and rename to the target file. The temporary file may exist from
+ * the last failed attempt (failed during partial padding or
+ * renaming or some other). If it exists, let's play safe and
+ * delete it before creating a new one.
+ */
+ snprintf(tmpsuffixpath, sizeof(tmpsuffixpath), "%s.%s",
+ targetpath, "temp");
+
+ if (dir_existsfile(tmpsuffixpath))
+ {
+ if (unlink(tmpsuffixpath) != 0)
+ {
+ dir_data->lasterrno = errno;
+ return NULL;
+ }
+ }
--
Bharath Rupireddy
RDS Open Source Databases: https://aws.amazon.com/rds/postgresql/
