Hi Michael,
> Here are two patches to address both issues:
Thank you for the preparing the patches and test simplification. My bad, I overcompilcated it.
I concerned about twophase file name generation. The function TwoPhaseFilePath() is pretty straitforward and
unambiguousin 16 and earlier versions. The logic of this function in 17+ seems to be more complex. I do not understand
itclearly. But, I guess, it will work incorrectly after turning to a newer epoch, because the epoch is calculated from
TransamVariables->nextXid,but not from problem xid. The same problem may happen if we are in epoch 1 or greater. It
willproduce a wrong file name, because the epoch will be obtained from the last xid, not from file name xid. In another
words,the function AdjustToFullTransactionId assumes that if xid > TransamVariables->nextXid, then the xid from the
previousepoch. I may be not the case in our scenario.
> I suspect that this can be still dangerous as-is while complicating the code with more possible paths for the removal
ofthe 2PC files
Agree, but we may pass file name into TwoPhaseFilePath if any, instead of creation of two functions as in the patch.
Thecost - two new if conditions. Using file names is pretty safe. Once we read the file and extract xid from its name,
justpass this file name to TwoPhaseFilePath(). If not, try to generate it. Anyway, I do not insist on it, just try to
discuss.
With best regards,
Vitaly
