On 12/13/2013 07:55 AM, Sawada Masahiko wrote:
On Fri, Dec 13, 2013 at 1:51 PM, Dilip kumar <dilip.ku...@huawei.com> wrote:
On 04 December 2013, Sawada Masahiko Wrote
I attached the patch which have modified based on Robert suggestion,
and fixed typo.
I have reviewed the modified patch and I have some comments..
1. Patch need to be rebased (failed applying on head)
2. crc field should be at end in ControlFileData struct, because for crc
calculation, it directly take the offset of crc field in ControlFileData.
/* CRC of all above ... MUST BE LAST! */
pg_crc32 crc;
+
+ /* Enable logging WAL when updating hint bits */
+ bool wal_log_hintbits;
} ControlFileData;
3. wal_log_hintbits field should be printed in PrintControlValues function.
Actually, no. It's reset anyway like wal_level and some other settings,
so it's not an interesting value to print out.
Thanks for the review!
I have modified the patch base on your comment, and I attached the v7 patch.
Thanks, committed with some minor changes:
The amount of extra WAL-logging with wal_log_hintbits=on is almost, but
not exactly the same as with checksums enabled. With checksums enabled,
visibilitymap_set() creates a full-page image of the heap page, but with
wal_log_hintbits=on, it does not. For pg_rewind's purposes, that's
correct, but I feel that it would be better if the decision on whether
to WAL-log or not was exactly the same in both cases. One reason is that
you could then use wal_log_hintbits=on to evaluate how big the impact on
WAL volume would be if you had checksums enabled. I committed it that way.
OTOH, for pg_rewind's purposes, there's actually no need to take a full
page image when a hint bit is set. A small WAL-record indicating that
the page was modified would be enough. It's particularly strange to
insist that hint bit updates create full-page images, when you have
full_page_writes=off so that normal updates don't create them.
We're a bit schizophrenic with full page writes also when data checksums
are enabled, though. If I'm reading the code correctly, you can turn
full_page_writes=off even when data checksums are enabled, which exposes
you to torn page problems. Which might be OK under some special
circumstances. But you'll still get full-page images of hint bits!
I think it's a bit excessive to require wal_level > minimal if you set
wal_log_hintbits=on. It's a bit silly to ask for wal_log_hintbits=on
without archiving, but I also don't see a big reason to throw an error.
It would be annoying, if you want to e.g temporarily set
wal_level=minimal when you do a big data load, and re-initialize the
standby afterwards. Now you'd also need to remember to turn
wal_log_hintbits=off temporarily, and remember to turn it back on
afterwards. So I just removed that check.
I'm not totally satisfied with the name of the GUC, wal_log_hintbits.
The term "hint bits" doesn't mean anything to most people. I couldn't
come up with anything better, but if someone has a better suggestion, we
can still change it.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
