"Koichi Suzuki" <koichi....@gmail.com> wrote: > > This patc also include additional patch for posix_fadvise to skip > > prefetch of pages which does not exist.
I reviewed your patch and found items to be fixed and improved. - You should not claim your copyrights. There are your copyright claims in two files newly added, but they should be included by PostgreSQL Global Development Group. | * Portions Copyright (c) 2008, Nippon Telegraph and Telephone Corporation | * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group - Avoid pending wal records on continuous REDO. In archive recovery, ReadRecord() waits until the next segment comes. Should we avoid pending WAL records during the waiting? RedoRecords() might be needed before calling RestoreArchivedFile(). It would be better to redo records and restore archived files in parallel, but we will need to replace system() to other system because system() blocks until the process finishes. - Should check the buffer pool before calling smgrprefetch. If the page is in the shared buffer pool, we don't need to prefetch the buffer from disks. I think it is good to add PrefetchBufferWithoutRelcache() like ReadBufferWithoutRelcache(). - Is possible to remove calls of ReadAheadHasRoom()? If we checks rooms of readahead-queue, xxx_readahead functions don't need to call ReadAheadHasRoom. The maximum readaheads are at most 3, so we'll flush the queue when the rooms of the queue are less than 3. And if the queue is full unexpectedly, we can ignore the added items because it is only hint. - Should avoid large static variables. | [src/backend/access/transam/xlog] | + static char RecordQueueBuf[XLogSegSize * 2]; | [readahead.c] | + static XLogReadAhead ReadAheadQueue[ReadAheadQueueSize]; I think it is a bad idea to place a large object as a static variable. It should be a pointer and allocated with palloc() in runtime. Also those variable are only used by startup process. - Merge readahead.h to xlog.h or something. It export just a few functions and the functionality belongs xlog and recovery. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
