Re: pg_rewind copies

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: David Steele <david(at)pgmasters(dot)net>, Cary Huang <cary(dot)huang(at)highgo(dot)ca>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pg_rewind copies
Date: 2022-04-01 09:00:00
Message-ID: FBE3D7A0-4291-40F1-B4B7-637A440686C7@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I took another look at this patch, and I think it's ready to go in, it clearly
fixes a bug that isn't too hard to hit in production settings. To ensure we
don't break this I've added a testcase which pipes the pg_rewind --verbose
output to a file it's asked to copy, which then guarantees that the file is
growing in size during the operation without need for synchronizing two
processes with IPC::Run (it also passes on Windows in the CI setup).

One small comment on the patch:

+ snprintf(srcpath, sizeof(srcpath), "%s/%s", datadir, path);

This should IMO check the returnvalue of snprintf to ensure it wasn't
truncated. While the risk is exceedingly small, a truncated filename might
match another existing filename and the error not getting caught. There is
another instance just like this one in open_target_file() to which I think we
should apply the same belts-and-suspenders treatment. I've fixed this in the
attached version which also have had a pg_indent run on top of a fresh rebase.

Thoughts on this version?

--
Daniel Gustafsson https://vmware.com/

Attachment Content-Type Size
v3-0001-pg_rewind-Fetch-small-files-according-to-new-size.patch application/octet-stream 14.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2022-04-01 09:00:08 Re: Use generation context to speed up tuplesorts
Previous Message Pavel Borisov 2022-04-01 08:58:35 Re: Unit tests for SLRU