Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> [questions about confusing (obsolete) comments]
> Setting the high bit in OldSetXidAdd() seems a bit weird. How
> about just using UINT64_MAX instead of 0 to mean no conflicts? Or
> 1, and start the sequence counter from 2.
> ReleasePredicateLocks() reads ShmemVariableCache->nextXid without
> XidGenLock. Maybe it's safe, we assume that TransactionIds are
> atomic elsewhere, but at least there needs to be a comment
> explaining it. But it probably should use ReadNewTransactionId()
> instead.
> Attached is a patch for some trivial changes, mostly typos.
> There's a few remaining TODO comments in the code, which obviously
> need to be resolved one way or another, but as soon as you're
> finished with any outstanding issues that you know of, I think
> this is ready for commit.
As of this moment, everything you explicitly mentioned is covered.
In addition, I've looked all over for lurking forgotten issues, and
found and dealt with a few small ones. Much to my chagrin, I found
one big one (interaction between this feature and hot standby) which
is being discussed on a separate thread; it's probably best not to
split the discussion between threads, so please don't reply
regarding that issue on this thread.
Some of the new code hasn't had very much testing beyond the make
targets: check, installcheck-world, and dcheck (new in this patch).
Dan did a few "crash the server between prepare and commit" tests,
but we need another round of stress-testing designed to beat up on
that part. I hand-tested some query sets to ensure that the "safe
retry" code is working as well as we can do without actively
interrupting the victim. (We simply flag the victim and it will
check the flag and kill itself the next time it hits certain
functions in SSI.)
The remaining "TODO SSI" comments are related to possible work for
future releases.
Other than needing a bit more testing, especially around 2PC and
"safe retry" the only outstanding issue is SSI+HS.
I'm attaching version 13 of the patch. I will be following up
shortly about the SSI+HS issue on the other thread. Clearly, that
will require at least one more version.
-Kevin