Hi,
On 2020-03-31 13:04:38 -0700, Andres Freund wrote:
> I'm still fighting with snapshot_too_old. The feature is just badly
> undertested, underdocumented, and there's lots of other oddities. I've
> now spent about as much time on that feature than on the whole rest of
> the patchset.
To expand on this being under-tested: The whole time mapping
infrastructure is not tested, because all of that is bypassed when
old_snapshot_threshold = 0. And old_snapshot_threshold = 0 basically
only exists for testing. The largest part of the complexity of this
feature are TransactionIdLimitedForOldSnapshots() and
MaintainOldSnapshotTimeMapping() - and none of the complexity is tested
due to the tests running with old_snapshot_threshold = 0.
So we have test only infrastructure that doesn't allow to actually test
the feature.
And the tests that we do have don't have a single comment explaining
what the expected results are. Except for the newer
sto_using_hash_index.spec, they just run all permutations. I don't know
how those tests actually help, since it's not clear why any of the
results are the way they are. And which just are the results of
bugs. Ore not affected by s_t_o.
Greetings,
Andres Freund
