pgsql: Write a WAL record whenever we perform an operation without
in [PgSql]
|
Prev: [HACKERS] New VACUUM FULL crashes on temp relations
Next: pgsql: Write a WAL record wheneverwe perform an operation without
From: Fujii Masao on 1 Feb 2010 04:58 On Mon, Feb 1, 2010 at 6:33 PM, Heikki Linnakangas <heikki.linnakangas(a)enterprisedb.com> wrote: > Hmm. The "unlogged" record is written here: > > ... > void > heap_sync(Relation rel) > { > char reason[NAMEDATALEN + 30]; > > /* temp tables never need fsync */ > if (rel->rd_istemp) > return; > > snprintf(reason, sizeof(reason), "heap inserts on \"%s\"", > RelationGetRelationName(rel)); > XLogReportUnloggedStatement(reason); > ... > > > So it clearly shouldn't be written for temp relations. Apparently the > rd_istemp flag not set correctly after CLUSTER / VACUUM FULL. The cause of the problem seems to be the new heap created by rebuild_relation() and copy_heap_data(), i.e., new VACUUM FULL. Since it's not a temporary heap, its rd_istemp is off. OTOH it needs to be synced after physical copy from old heap. So XLogReportUnloggedStatement() is called in heap_sync(). The easy fix is to change the code as below. if (XLogIsNeeded()) { snprintf(reason, sizeof(reason), "heap inserts on \"%s\"", RelationGetRelationName(rel)); XLogReportUnloggedStatement(reason); } But I'm not sure this fix is right, so I need to investigate the code more. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Fujii Masao on 1 Feb 2010 05:15 On Mon, Feb 1, 2010 at 6:58 PM, Fujii Masao <masao.fujii(a)gmail.com> wrote: > if (XLogIsNeeded()) > { > snprintf(reason, sizeof(reason), "heap inserts on \"%s\"", > RelationGetRelationName(rel)); > XLogReportUnloggedStatement(reason); > } Oops! Typo: XLogIsNeeded() --> !XLogIsNeeded() Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Fujii Masao on 1 Feb 2010 05:36 On Mon, Feb 1, 2010 at 7:27 PM, Heikki Linnakangas <heikki.linnakangas(a)enterprisedb.com> wrote: > Fujii Masao wrote: >> The cause of the problem seems to be the new heap created by >> rebuild_relation() and copy_heap_data(), i.e., new VACUUM FULL. >> Since it's not a temporary heap, its rd_istemp is off. OTOH >> it needs to be synced after physical copy from old heap. > > Why does it need to be synced? > > ISTM the bug is that rd_istemp is off at that point. Umm... ISTM that new heap needs to be synced before calling swap_relation_files(), but, in the now, I'm not sure whether it's really required or not. Sorry. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Fujii Masao on 1 Feb 2010 05:38 On Mon, Feb 1, 2010 at 7:30 PM, Heikki Linnakangas <heikki.linnakangas(a)enterprisedb.com> wrote: > CREATE TEMPORARY TABLE foo (id int4); > VACUUM FULL foo; How about issuing just "VACUUM FULL" after creating log-shipping environment? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 1 Feb 2010 11:13
Fujii Masao <masao.fujii(a)gmail.com> writes: > On Mon, Feb 1, 2010 at 7:27 PM, Heikki Linnakangas > <heikki.linnakangas(a)enterprisedb.com> wrote: >> Fujii Masao wrote: >>> The cause of the problem seems to be the new heap created by >>> rebuild_relation() and copy_heap_data(), i.e., new VACUUM FULL. >>> Since it's not a temporary heap, its rd_istemp is off. OTOH >>> it needs to be synced after physical copy from old heap. >> >> Why does it need to be synced? >> >> ISTM the bug is that rd_istemp is off at that point. > Umm... ISTM that new heap needs to be synced before calling > swap_relation_files(), but, in the now, I'm not sure whether > it's really required or not. Sorry. If the original table is temp, then none of this work needs to be fsync'd. Ever. So ISTM that CLUSTER ought to be copying the istemp bit. Another reason to do that is to make sure that the new instance of the table goes into the temp tablespace and not the regular one. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |