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 takingadvantage that the lsn for such pages stays 0/0. I'd like to
seeacomment mentioning for, say, buffer_capture_is_changed? orbuffer_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
noinformation.LSN, or quadraplet of LSN, rnode, forknum andblockno seems to be more informative.
- buffer_capture_is_changed()
The name for the function semes to be misleading. What thisfunction does is comparing LSNs between stored page image
andcurrentpage. lsn_is_changed(BufferImage) or something likewould be more clearly.
====== bufmgr.c:
- ConditionalLockBuffer()
Sorry for a trivial comment, but the variable 'res' concealesthe meaning. "acquired" seems to be more preferable, isn't
it?
- LockBuffer()
There is a 'XXX' comment. The discussion written there seems tobe right, for me. If you mind that peeking into there
isa badbehavior, 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
PGPORTsemes to be 65535 but this script search for free port up to the number larger by 16 from the start, which would
bebeyond 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
mentionthe port numbers, it might should be 'port $PGPORTSTART-$PGPORT ..' or would be 'All 16 ports attempted failed'
orsomething..
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center