Re: Corner-case bug in pg_rewind
| От | Pavel Borisov |
|---|---|
| Тема | Re: Corner-case bug in pg_rewind |
| Дата | |
| Msg-id | CALT9ZEFgPOZF6eua9MB7tN0ZZQjTxJL=qj0sKeO1YyPPZBEO4g@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Corner-case bug in pg_rewind (Heikki Linnakangas <hlinnaka@iki.fi>) |
| Ответы |
Re: Corner-case bug in pg_rewind
|
| Список | pgsql-hackers |
I did some effort to review your patch which seems legit to me.
I think some minor things are better to be improved i.e.
1. Comment regarding
------
347 * Check for the possibility that the target is in fact a direct
348 * ancestor of the source. In that case, there is no divergent history349 * in the target that needs rewinding.
------
are better be reformulated as overall block contents are mostly cover vice versa case of a target NOT being a direct ancestor of the source. Maybe: "We need rewind in cases when .... and don't need only if the target is a direct ancestor of the source." I think it will be more understandable if it would be a commentary with descriptions of all cases in the block or no commentary before the block at all with commentaries of these cases on each if/else inside the block instead.
2. When I do the test with no patching of pg_rewind.c I get the output:
-----
# Failed test 'pg_rewind detects rewind needed stderr /(?^:rewinding from last common checkpoint at)/'# at t/007_min_recovery_point.pl line 107.
# 'pg_rewind: servers diverged at WAL location 0/3000180 on timeline 2
# pg_rewind: no rewind required
# '
# doesn't match '(?^:rewinding from last common checkpoint at)'
# Looks like you failed 1 test of 2.
t/007_min_recovery_point.pl .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/2 subtests
Test Summary Report
-------------------
t/007_min_recovery_point.pl (Wstat: 256 Tests: 2 Failed: 1)
Failed test: 2
Non-zero exit status: 1
-------
Maybe it can just give "failed" without so many details?
Also, I think Heikki's notion could be fulfilled.
Apart from this I consider the patch clean, clear, tests are passes and I'd recommend it to commit after a minor improvement described.
Thank you!
--
В списке pgsql-hackers по дате отправления: