Re: non-HOT update not looking at FSM for large tuple update

On Tue, 9 Mar 2021 at 18:39, John Naylor <john.naylor@enterprisedb.com> wrote:
>
> I wrote:
>
> > That seems like the proper fix, and I see you've started a thread for that. I don't think that change in behavior would be backpatchable, but patch here might have a chance at that.
>
> I remembered after the fact that truncating line pointers would only allow for omitting the 2% slack logic (and has other benefits), but the rest of this patch would be needed regardless.

Regarding the 2% slack logic, could we change it to use increments of
line pointers instead? That makes it more clear what problem this
solution is trying to work around; that is to say line pointers not
(all) being truncated away. The currently subtracted value accounts
for the size of 40 line pointers on 8k-pages (~ 13.7% of
MaxHeapTuplesPerPage), and slightly higher fractions (up to 13.94%)
for larger page sizes. As the to-be-inserted tuple is already _at
least_ 10% of MaxHeapTupleSize when it hits this new code.

Also, even with this patch, we do FSM-requests of for sizes between
MaxHeapTupleSize - 2% and MaxHeapTupleSize, if len+saveFreeSpace falls
between those two numbers. I think we better clamp the fsm request
between `len` and `MaxHeapTupleSize - PAGE_SIZE_DEPENDENT_FACTOR`.

So, I sugges the following incremental patch:

 bool       needLock;
+ const Size maxPaddedFsmRequest = MaxHeapTupleSize -
(MaxHeapTuplesPerPage / 8 * sizeof(ItemIdData));
...
- if (len + saveFreeSpace > MaxHeapTupleSize)
+ if (len + saveFreeSpace > maxPaddedFsmRequest)
...
- targetFreeSpace = Max(len, MaxHeapTupleSize - (MaxHeapTupleSize * 2 / 100));
+ targetFreeSpace = Max(len, maxPaddedFsmRequest);
...

Other than this, I think this is a good fix.


With regards,

Matthias van de Meent.