Re: pg_xlogdump follow into the future

Поиск
Список
Период
Сортировка
От Haribabu Kommi
Тема Re: pg_xlogdump follow into the future
Дата
Msg-id CAJrrPGd1z8ayge6owfvAWWHrPXGFDWq_Ns10L-U+P7r_GC7_rw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: pg_xlogdump follow into the future  (Vladimir Rusinov <vrusinov@google.com>)
Ответы Re: [HACKERS] pg_xlogdump follow into the future  (Michael Paquier <michael.paquier@gmail.com>)
Список pgsql-hackers


On Fri, Dec 2, 2016 at 2:21 AM, Vladimir Rusinov <vrusinov@google.com> wrote:
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.

Summary
=======

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 db/pg_wal/000000010000000000000001`.
This spewed quite a lot of stuff, as expected.

2) I've connected to the same instance and ran following:
# SELECT pg_switch_xlog();
 pg_switch_xlog 
----------------
 0/14FA3D8
(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
===============
n/a

Coding review
===========
LGTM

Architecture review
==============
n/a

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.

Regards,
Hari Babu
Fujitsu Australia

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Haribabu Kommi
Дата:
Сообщение: Re: VACUUM's ancillary tasks
Следующее
От: Haribabu Kommi
Дата:
Сообщение: Re: WAL logging problem in 9.4.3?