Re: Unlogged tables cleanup
От | Robert Haas |
---|---|
Тема | Re: Unlogged tables cleanup |
Дата | |
Msg-id | CA+TgmoZHNu1mrbuqT=sgpHDUSMRDnYWHUpM2PVM-NCz4GY-44Q@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Unlogged tables cleanup (Michael Paquier <michael.paquier@gmail.com>) |
Ответы |
Re: Unlogged tables cleanup
(Michael Paquier <michael.paquier@gmail.com>)
|
Список | pgsql-hackers |
On Thu, Nov 10, 2016 at 9:25 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Thu, Nov 10, 2016 at 10:52 PM, Kuntal Ghosh > <kuntalghosh.2007@gmail.com> wrote: >> On Thu, Nov 10, 2016 at 3:42 PM, Michael Paquier >> <michael.paquier@gmail.com> wrote: >>> Nah. Looking at the code the fix is quite obvious. >>> heap_create_init_fork() is checking for XLogIsNeeded() to decide if >>> the INIT forknum should be logged or not. But this is wrong, it needs >>> to be done unconditionally to ensure that the relation gets created at >>> replay. >> >> I think that we should also update other *buildempty() functions as well. >> For example, if the table has a primary key, we'll encounter the error again >> for btree index. > > Good point. I just had a look at all the AMs: bloom, btree and SP-gist > are plainly broken. The others are fine. Attached is an updated patch. > > Here are the tests I have done with wal_level = minimal to test all the AMs: > \! rm -rf /Users/mpaquier/Desktop/tbsp/PG* > create extension bloom; > create extension btree_gist; > create extension btree_gin; > create tablespace popo location '/Users/mpaquier/Desktop/tbsp'; > set default_tablespace = popo; > create unlogged table foo (a int); > insert into foo values (generate_series(1,10000)); > create index foo_bt on foo(a); > create index foo_bloom on foo using bloom(a); > create index foo_gin on foo using gin (a); > create index foo_gist on foo using gist (a); > create index foo_brin on foo using brin (a); > create unlogged table foo_geo (a box); > insert into foo_geo values ('(1,2),(2,3)'); > create index foo_spgist ON foo_geo using spgist(a); > > -- crash PG > -- Insert some data > insert into foo values (1000000); > insert into foo_geo values ('(50,12),(312,3)'); > > This should create 8 INIT forks, 6 for the indexes and 2 for the > tables. On HEAD, only 3 are getting created and with the patch all of > them are. The header comment for heap_create_init_fork() says this: /** Set up an init fork for an unlogged table so that it can be correctly* reinitialized on restart. Since we're going todo an immediate sync, we* only need to xlog this if archiving or streaming is enabled. And the* immediate sync is required,because otherwise there's no guarantee that* this will hit the disk before the next checkpoint moves the redo pointer.*/ Your patch causes the code not to match the comment any more. And the comment explains why at the time I wrote this code I thought it was OK to have the XLogIsNeeded() test in there, so it clearly needs updating. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
В списке pgsql-hackers по дате отправления:
Следующее
От: Alvaro HerreraДата:
Сообщение: Re: [COMMITTERS] pgsql: Build HTML documentation using XSLT stylesheets by default