Re: WAL format and API changes (9.5)

On 2014-04-04 12:50:25 +0300, Heikki Linnakangas wrote:
> On 04/04/2014 11:41 AM, Andres Freund wrote:
> >On 2014-04-04 10:48:32 +0300, Heikki Linnakangas wrote:
> >>@@ -484,10 +483,11 @@ PageRepairFragmentation(Page page)
> >> ((PageHeader) page)->pd_upper = pd_special;
> >> }
> >> else
> >>- { /* nstorage != 0 */
> >>+ {
> >> /* Need to compact the page the hard way */
> >>- itemidbase = (itemIdSort) palloc(sizeof(itemIdSortData) * nstorage);
> >>- itemidptr = itemidbase;
> >>+ itemIdSortData itemidbase[MaxHeapTuplesPerPage];
> >>+ itemIdSort itemidptr = itemidbase;
> >>+
> >
> >That's a fair bit of stack, and it can be called somewhat deep on the
> >stack via heap_page_prune_opt(). I wonder if we ought to add a
> >check_stack_depth() somewhere.
>
> Hmm, on my 64-bit laptop, that array is 24*291=6984 bytes with 8k block
> size. That's fairly large, but not unheard of; there are a few places where
> we allocate a BLCKSZ-sized buffer from stack.

Yea, I am not complaing about using so much stack. Seems sensible here.

> But overall I wouldn't worry about it. check_stack_depth() leaves a fair
> amount of headroom: STACK_DEPTH_SLOP is 512kB. As long as we don't recurse,
> that's plenty.

Well, there's no checks at nearby afair. That's why I was
wondering... But I don't have a big problem with not checking, I just
wanted to bring it up.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services