Hello, This is the additional comments for other part. I haven't see significant defect in the code so far.
===== bufcapt.c: - buffer_capture_remember() or so. Pages for unlogged tables are avoided to be written taking advantage that the lsn for such pages stays 0/0. I'd like to see a comment mentioning for, say, buffer_capture_is_changed? or buffer_capture_forget or somewhere. - buffer_capture_forget() However this error is likely not to occur, in the error message "could not find image...", the buffer id seems to bring no information. LSN, or quadraplet of LSN, rnode, forknum and blockno seems to be more informative. - buffer_capture_is_changed() The name for the function semes to be misleading. What this function does is comparing LSNs between stored page image and current page. lsn_is_changed(BufferImage) or something like would be more clearly. ====== bufmgr.c: - ConditionalLockBuffer() Sorry for a trivial comment, but the variable 'res' conceales the meaning. "acquired" seems to be more preferable, isn't it? - LockBuffer() There is a 'XXX' comment. The discussion written there seems to be right, for me. If you mind that peeking into there is a bad behavior, adding a macro into lwlock.h would help:p lwlock.h: #define LWLockHoldedExclusive(l) ((l)->exclusive > 0) lwlock.h: #define LWLockHoldedShared(l) ((l)->shared > 0) # I don't see this usable so much.. bufmgr.c: if (LWLockHoldedExclusive(buf->content_lock)) If there isn't any particular concern, 'XXX:' should be removed. ===== bufpage.c: - #include "storage/bufcapt.h" The include seems to be needless. ===== bufcapt.h: - header comment The file name is misspelled as 'bufcaptr.h'. Copyright notice of UC is needed? (Other files are the same) - The includes in this header except for buf.h seem not to be necessary. ===== init_env.sh: - pg_get_test_port() It determines server port using PG_VERSION_NUM, but is it necessary? Addition to it, the theoretical maximum initial PGPORT semes to be 65535 but this script search for free port up to the number larger by 16 from the start, which would be beyond the edge. - pg_get_test_port() It stops with error after 16 times of failure, but the message complains only about the final attempt. If you want to mention the port numbers, it might should be 'port $PGPORTSTART-$PGPORT ..' or would be 'All 16 ports attempted failed' or something.. regards, -- Kyotaro Horiguchi 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
