From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
---|---|
To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
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 10:46:01 |
Message-ID: | 994e8f27-2eb6-b4b4-d7eb-5619821f1306@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 01/04/2022 12:00, Daniel Gustafsson wrote:
> 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.
> + if (len >= sizeof(dstpath))
> + pg_fatal("filepath buffer too small"); /* shouldn't happen */
Makes sense. I would remove the "shouldn't happen"; it's not very hard
to make it happen, you just need a very long target datadir path. And
rephrase the error message as "datadir path too long".
One typo in the commit message: s/update/updates/.
Thanks!
- Heikki
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2022-04-01 11:46:19 | Re: pg_rewind copies |
Previous Message | David Rowley | 2022-04-01 09:00:08 | Re: Use generation context to speed up tuplesorts |