On Mon, 8 Mar 2021 at 16:25, Floris Van Nee <florisvan...@optiver.com> wrote: > > > I've added this to the commitfest as a bug fix and added you as an author. > > Thanks. Patch looks good to me, but I guess there needs to be someone else > reviewing too? > Also, would this be a backpatchable bugfix? > > -Floris >
This patch fails to consider that len may be bigger than MaxHeapTupleSize * 0.98, which in this case triggers a reproducable PANIC: =# CREATE TABLE t_failure (a int, b text) WITH (fillfactor = 10); -- force the new FSM calculation for large tuples CREATE TABLE =# ALTER TABLE t_failure ALTER COLUMN b SET STORAGE plain; ALTER TABLE =# INSERT INTO t_failure (SELECT FROM generate_series(1, 32)); -- use up 32 line pointers on the first page. INSERT 0 32 =# DELETE FROM t_failure; DELETE 32 =# VACUUM (TRUNCATE OFF) t_failure; -- we now have a page that has MaxHeapTupleSize > free space > 98% MaxHeapTupleSize VACUUM =# INSERT INTO t_failure (select 1, string_agg('1', '') from generate_series(1, 8126)); PANIC: failed to add tuple to page server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. A possible solution should always request at least the size of the requested tuple, e.g.: - targetFreeSpace = MaxHeapTupleSize - (MaxHeapTupleSize * 2 / 100); + targetFreeSpace = Max(len, MaxHeapTupleSize - (MaxHeapTupleSize * 2 / 100)); One different question I have, though, is why we can't "just" teach vacuum to clean up trailing unused line pointers. As in, can't we trim the line pointer array when vacuum detects that the trailing line pointers on the page are all unused? The only documentation that I could find that this doesn't happen is in the comment on PageIndexTupleDelete and PageRepairFragmentation, both not very descriptive on why we can't shrink the page->pd_linp array. One is "Unlike heap pages, we compact out the line pointer for the removed tuple." (Jan. 2002), and the other is "It doesn't remove unused line pointers! Please don't change this." (Oct. 2000), but I can't seem to find the documentation / conversations on the implications that such shrinking would have. With regards, Matthias van de Meent.