Re: Fix bug in multixact Oldest*MXactId initialization and access

>> There would no the bug. There would no this discussion.
>> If only array bounds were checked with asserts.

> Yeah, more asserts == good, usually.

Maybe. My thoughts were that If the MaxOldestSlot is set correctly at
initialization time,
and we guarantee that by asserting its size against the total number
of procs, that should
be sufficient, and we don't need to create wrapper functions or add
assertions on every
access. But. I don't have a strong opinion either way.

Anyhow, it looks like the proposals now are aiming towards eliminating
the need to account
for auxiliary procs, and that is probably OK. It's a small bit of
memory saved, but slightly bit
more code.

> With extremely low configured MaxBackends it will overflow to
> BufferDescriptors array. Certainly only test configurations may use such
> tiny settings.
>
> I found the bug because I increased NUM_AUXILIARY_PROCS by bunch of other
> specialized processes, so overflow to BufferDescriptors happened even with
> parameters already set in tests.

It's unfortunately a bit worse. Here is a repro that shows 2 prepared
transactions
being created after a shared lock is taken on a table with 1 row. A subsequent
delete is able to complete, where we would expect it to be blocked until the
prepared transactions COMMIT or ROLLBACK.

With simply adding NUM_AUXILIARY_PROCS to MaxOldestSlot, it works as expected,
and the delete is blocked.

```
-#define MaxOldestSlot (MaxBackends + max_prepared_xacts)
+#define MaxOldestSlot (MaxBackends + NUM_AUXILIARY_PROCS + max_prepared_xacts)
```

I also tried with Heikki's proposal and the test succeeded. I did not
review the patch thoroughly
yet, but I think this test case should be added.

The test does require 2 prepared transactions to exercise
MultiXactIdSetOldestVisible(),
which then results in reading garbage values from the array and
determining incorrect
visibility of the row.

--
Sami Imseih
Amazon Web Services (AWS)