On Fri, Oct 21, 2022 at 10:43 AM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > Thanks, but we don't need to wipe out the all bytes. Just putting \0 > at the beginning of the buffer is sufficient.
Nah, that's not a clean way IMO. > And the Memset() at the > beginning of basic_archive_file_internal is not needed since that > static variables are initially initialized to zeros. Removed. MemSet() after durable_rename() would be sufficient. > This is not necessarily needed, but it might be better we empty > tempfilepath after unlinking the file. I think it's not necessary as the archiver itself is shutting down and I don't think the server calls the shutdown callback twice. However, if we want basic_archive_shutdown() to be more protective against multiple calls (for any reason that we're missing), we can have a static local variable to quickly exit if the callback is already called. instead of MemSet(), but that's not needed I guess. > + expectation that a value will soon be provided. Care must be taken when > + multiple servers are archiving to the same > + <varname>basic_archive.archive_library</varname> directory as they all > + might try to archive the same WAL file. > > I don't understand what kind of care should be taken by reading this.. It's just a notice, however I agree with you that it may be confusing. I've removed it. Please review the attached v4 patch. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
v4-0001-Remove-leftover-temporary-files-via-basic_archive.patch
Description: Binary data
