Re: Re: PANIC: invalid index offnum: 186 when processing BRIN indexes in VACUUM

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
>
> > Yeah, I think this approach results in better code. The attached patch
> > implements that, and it passes the test for me (incl. calling
> > brin_summarize_new_values concurrently with vacuum, when running the
> > insert; the former does include the final page range whereas the latter
> > does not.)
>
> Hm, so IIUC the point is that once the placeholder tuple is in, we can
> rely on concurrent inserters to update it for insertions into pages that
> are added after we determine our scan stop point. But if the scan stop
> point is chosen before inserting the placeholder, then we have a race
> condition.

Exactly. We don't need to scan those pages once the placeholder tuple
is in.

> The given code seems a brick or so short of a load, though: if the table
> has been extended sufficiently, it could compute scanNumBlks as larger
> than bs_pagesPerRange, no? You need to clamp the computation result.

Oops, right.

> Also, shouldn't the passed-in heapBlk always be a multiple of
> pagesPerRange already?

Yeah, I guess I can turn that into an assert.

> Do we still need the complication in brinsummarize to discriminate
> against the last partial range? Now that the lock consideration
> is gone, I think that might be a wart.

You mean this code?

/*
* Unless requested to summarize even a partial range, go away now if
* we think the next range is partial.
*
* Maybe the table already grew to cover this range completely, but we
* don't want to spend a whole RelationGetNumberOfBlocks to find out,
* so just leave it for next time.
*/
if (!include_partial &&
(startBlk + pagesPerRange > heapNumBlocks))
break;

In the case of VACUUM, it's not desirable to create a summarization for
the last partial range, because if the table is still being filled, that
would slow down the insertion process. So we pass include_partial=false
there. In brin_summarize_new_values, the theory is that the user called
that function because they're done loading (at least temporarily) so
it's better to process the partial range.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services