Hi Srinath,
Thank you for the quick review!
On Sat, Oct 18, 2025 at 9:05 AM Srinath Reddy Sadipiralla
<srinath2133@gmail.com> wrote:
> ...
> I agree with Robert here, and i think if we found
> that the WAL filename is not valid after parsing
> we can return something like FILE_CONTENT_INVALID_TYPE
> in getFileContentType so that it won't end up going
> through decide_wal_file_action,then XLogFromFileName
> may assign file_segno invalid values greater than
> last_common_segno which means we lead to copying it,even
> though it's an invalid file, thoughts?
>
My understanding of pg_rewind is that it aims to make the two clusters
"identical"
after rewind, similar to a basebackup. It does not try to make decisions
on whether or not a file is necessary for database to start. I'd
prefer the patch aims
to keep parity with the existing behavior of pg_rewind/pg_basebackup as much as
possible. If there is an unexpected/invalid segment and we can't make
assumptions
about the content, we should just sync it to keep with existing behavior.
There's lots of potential optimizations pg_rewind can do if we only
talk about what is
"strictly" necessary for the database to start. We can exclude any
directories that
should not be there. For example, if there exists a directory
$PGDATA/random_files
on source and target, the whole directory gets synced right now. We could not
include that directory and the database would most likely still start
but that goes
against the current way pg_rewind works.
> 2) Please fix the indentation by running pgindent,
> also add file_content_type_t typedef to typedefs.list
> file.
Will update.
> 3) maybe we can improve the error messages for the
> last 2 checks in tap test, that because of this
> reason hence proven that the copy from source to
> target has been done.
What are you thinking of? Something like the below?
+ "Expected WAL segment $corrupt_wal_seg to have been modified
due to rewind");
+ "Expected WAL segment $corrupt_wal_seg to have been synced
from source to target after rewind");
> 4) We need to update the pg_rewind docs regarding
> this new behaviour.
>
Will update.
--
John Hsu - Amazon Web Services
