On Fri, Dec 2, 2016 at 2:21 AM, Vladimir Rusinov <vrusi...@google.com>
> This patch does not have a reviewer, so I've decided to try myself on.
> Disclaimer: although I review quite a lot of code daily, this is my first
> review for PostgreSQL. I don't know code very well, and frankly I don't
> really know C very well.
> Hope my effort are not vain and will be helpful to somebody.
> I'll be happy for review on review and any tips on process.
> I favour this patch. Current behaviour is indeed confusing. If we keep
> current behaviour we need to update docs and maybe also print a warning
> when using -f with a file name.
> Thank you for submission, but i'm afraid there is a bit more work here:
> - There is a bug, making it hard to test. Please fix.
> - Please add some tests.
> Submission review
> Patch applies cleanly on HEAD. Tests succeed.
> There are no new or affected by this patch tests. Given that I've found a
> trivial bug (see below), a test should be created.
> Usability review
> I believe I've immediately hit a corner case:
> 1) I've created a new instance, started it and run `./bin/pg_xlogdump -f
> This spewed quite a lot of stuff, as expected.
> 2) I've connected to the same instance and ran following:
> # SELECT pg_switch_xlog();
> (1 row)
> xlogdump immediately crashed with following:
> pg_xlogdump: FATAL: could not find file "000000010000000000000002": No
> such file or directory
> Problem is that Postgres does not create file until it's actually needed.
> So 000000010000000000000002 indeed was not there until after I've run some
> transactions. I believe same would happen after pg_start_backup(), etc.
> Feature review
> See above. Can't do more testing.
> Performance review
> Coding review
> Architecture review
Patch received feedback at the end of commitfest.
Closed in 2016-11 commitfest with "moved to next CF".
Please feel free to update the status once you submit the updated patch.