Problem with repalloc downsizing patch

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Problem with repalloc downsizing patch
Дата
Msg-id 17059.1570208426@sss.pgh.pa.us
обсуждение исходный текст
Ответы Re: Problem with repalloc downsizing patch  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
skink just found what seems like a serious problem with commit
c477f3e44:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2019-10-04%2010%3A15%3A05

performing post-bootstrap initialization ... TRAP: FailedAssertion("newlist == list", File:
"/home/andres/build/buildfarm/HEAD/pgsql.build/../pgsql/src/backend/nodes/list.c",Line: 200) 
Aborted

That assert is here:

        /*
         * Currently, asking aset.c to reduce the allocated size of the List
         * header is pointless in terms of reclaiming space, unless the list
         * is very long.  However, it seems worth doing anyway to cause the
         * no-longer-needed initial_elements[] space to be cleared in
         * debugging builds.
         */
        newlist = (List *) repalloc(list, offsetof(List, initial_elements));

        /* That better not have failed, nor moved the list header */
        Assert(newlist == list);

If we invoke realloc() then it's very much within its rights to return
a block that's not where the original block was.  I'm a bit surprised
that initdb runs any SQL commands that create Lists long enough to
reach the threshold where c477f3e44 would make a difference (~1000
initial elements would be needed), but there you have it.

This is a bit sticky to resolve.  I'm inclined to think that the fault
is on list.c, as it really shouldn't assume that repalloc doesn't move
the block.  (In fact, the quoted comment is self-contradictory, since
it evidently thinks that space *could* be given back when a large
enough block is involved; which is exactly what didn't happen before
c477f3e44.)  But I do not think we can give up the invariant that List
headers don't move around.  Hence we basically can't do repalloc here
after all.  Perhaps, to limit the cost of that, we should eat the cost
of separately palloc'ing the ListCell array when the initial list length
exceeds some threshold.

I'm also wondering a bit whether there's anyplace *else* that is
cheating by assuming that a downsizing repalloc doesn't move the block.
We could investigate that by testing with a modified form of
AllocSetRealloc that always moves the block, but of course that won't
find bugs in untested code paths.  So another possibility is to revert
c477f3e44 and then document that AllocSetRealloc does not move a block
when reducing its size.  That does not seem very attractive though.

Any opinions about this?

            regards, tom lane



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Pavel Stehule
Дата:
Сообщение: Re: dropdb --force
Следующее
От: Ashwin Agrawal
Дата:
Сообщение: Re: Proposal: Make use of C99 designated initialisers fornulls/values arrays