Heikki Linnakangas wrote:
> Nothing checking for the hi-bit flag AFAICS. I guess the code that
> uses that would do check it.
Right. After getting this layer done, I went off to watch the
Badgers in the Rose Bowl, leaving that coding for today. ;-)
> But wouldn't it be simpler to mark the unused slots with zero
> commitseqno, instead of messing with the hi-bit in valid values?
This is the earliest commitSeqNo of rw-conflicts out which the
transaction we're looking up had. I'm using zero to mean that there
was no conflict. Perhaps instead of setting the high bit I could
just use a special value (like all bits set) instead of zero to mean
"no conflict". In any event, it's clear that all zero should mean
"not found" and I need some other way to indicate "no conflict".
> It's probably not necessary to explicitly truncate the slru at
> startup. We don't do that for pg_subtrans, which also doesn't
> survive restarts. The next checkpoint will truncate it.
Good point. That slims things down by 22 lines and eliminates a
distracting special case.
> It would possibly be simpler to not reset headXid and tailXid to
> InvalidTransactionId when the "window" is empty, but represent that
> as tailXid == headXid + 1.
I'll take a look. I went 'round a few time on how best to handle the
empty window, which was complicated a little bit by wanting to keep
track of the tail even when the window was currently empty. Because
xids won't be submitted in strictly sequential order, I might need
to go back a ways in the sequence to update something, so I need to
keep track of existing segment files even when there are currently no
xids to track; and I wanted the searches to have a fast path out for
such cases.
> OldSerXidGetMinConflictCommitSeqNo() calls LWLockRelease twice.
That's because the function calls SimpleLruReadPage_ReadOnly:
"Control lock must NOT be held at entry, but will be held at exit."
That strikes me as an odd API, but it is what it is.
-Kevin
