On Sun, Apr 10, 2022 at 11:16 PM Kyotaro Horiguchi <horikyota....@gmail.com> wrote:
> Sorry for the terrible typos.. > > At Sat, 9 Apr 2022 18:03:01 +0530, Bharath Rupireddy < > bharath.rupireddyforpostg...@gmail.com> wrote in > > On Tue, Jan 4, 2022 at 1:40 AM SATYANARAYANA NARLAPURAM > > <satyanarlapu...@gmail.com> wrote: > > > > > > On Sun, Jan 2, 2022 at 11:56 PM Michael Paquier <mich...@paquier.xyz> > wrote: > > >> Are you referring to the pre-padding when creating a new partial > > >> segment, aka when we write chunks of XLOG_BLCKSZ full of zeros until > > >> the file is fully created? What kind of error did you see? I guess > > >> that a write() with ENOSPC would be more likely, but you got a > > >> different problem? > > > > > > I see two cases, 1/ when no space is left on the device and 2/ when > the process is taken down forcibly (a VM/container crash) > > > > Yeah, these cases can occur leaving uninitialized .partial files which > > can be a problem for both pg_receivewal and pg_basebackup that uses > > dir_open_for_write (CreateWalDirectoryMethod). > > > > >> I don't disagree with improving such cases, but we > > >> should not do things so as there is a risk of leaving behind an > > >> infinite set of segments in case of repeated errors > > > > > > Do you see a problem with the proposed patch that leaves the files > behind, at least in my testing I don't see any files left behind? > > I guess that Michael took this patch as creating a temp file with a > name such like "tmp.n" every time finding an incomplete file. > > > With the proposed patch, it doesn't leave the unpadded .partial files. > > Also, the v2 patch always removes a leftover .partial.temp file before > > it creates a new one. > > > > >> , and partial > > >> segments are already a kind of temporary file. > > I'm not sure this is true for pg_receivewal case. The .partial file > is not a temporary file but the current working file for the tool. > Correct. The idea is to make sure the file is fully allocated before treating it as a current file. > > > > if the .partial file exists with not zero-padded up to the wal segment > size (WalSegSz), then open_walfile fails with the below error. I have two > options here, 1/ to continue padding the existing partial file and let it > zero up to WalSegSz , 2/create a temp file as I did in the patch. I thought > the latter is safe because it can handle corrupt cases as described below. > Thoughts? > > I think this patch shouldn't involve pg_basebackup. I agree to Cary > that deleting the erroring file should be fine. > > We already "skipping" (complete = non-.partial) WAL files with a wrong > size in FindStreamingStart so we can error-out with suggesting a hint. > > $ pg_receivewal -D xlog -p 5432 -h /tmp > pg_receivewal: error: segment file "0000000100000022000000F5.partial" has > incorrect size 8404992 > hint: You can continue after removing the file. > The idea here is to make pg_receivewal self sufficient and reduce human/third party tool interaction. Ideal case is running pg_Receivewal as a service for wal archiving. > > regards. > > -- > Kyotaro Horiguchi > NTT Open Source Software Center >
