Обсуждение: MinIndexTupleSize seems slightly wrong
itup.h says of MinIndexTupleSize/MaxIndexTuplesPerPage:
/*
* MaxIndexTuplesPerPage is an upper bound on the number of tuples that can
* fit on one index page. An index tuple must have either data or a null
* bitmap, so we can safely assume it's at least 1 byte bigger than a bare
* IndexTupleData struct. We arrive at the divisor because each tuple
* must be maxaligned, and it must have an associated item pointer.
*/
#define MinIndexTupleSize MAXALIGN(sizeof(IndexTupleData) + 1)
#define MaxIndexTuplesPerPage \
((int) ((BLCKSZ - SizeOfPageHeaderData) / \
(MAXALIGN(sizeof(IndexTupleData) + 1) + sizeof(ItemIdData))))
However, that at least seems questionable to me. See _bt_pgaddtup()
for a simple example of this -- "minus infinity" items on internal
pages are sized sizeof(IndexTupleData).
The code still seems fine to me, since that only happens at most once
per page. Is it worth noting the exception?
--
Peter Geoghegan
At Thu, 12 Apr 2018 17:26:33 -0700, Peter Geoghegan <pg@bowt.ie> wrote in <CAH2-WzkQmb54Kbx-YHXstRKXcNc+_87jwV3DRb54xcybLR7Oig@mail.gmail.com> > itup.h says of MinIndexTupleSize/MaxIndexTuplesPerPage: > > /* > * MaxIndexTuplesPerPage is an upper bound on the number of tuples that can > * fit on one index page. An index tuple must have either data or a null > * bitmap, so we can safely assume it's at least 1 byte bigger than a bare > * IndexTupleData struct. We arrive at the divisor because each tuple > * must be maxaligned, and it must have an associated item pointer. > */ > #define MinIndexTupleSize MAXALIGN(sizeof(IndexTupleData) + 1) > #define MaxIndexTuplesPerPage \ > ((int) ((BLCKSZ - SizeOfPageHeaderData) / \ > (MAXALIGN(sizeof(IndexTupleData) + 1) + sizeof(ItemIdData)))) > > However, that at least seems questionable to me. See _bt_pgaddtup() > for a simple example of this -- "minus infinity" items on internal > pages are sized sizeof(IndexTupleData). > > The code still seems fine to me, since that only happens at most once > per page. Is it worth noting the exception? MinIndexTupleSize is not used at all. It doesn't harm as long as MaxIndexTuplesPerPage is not less than the maximum number of tuples in reality. Applying values on my environment, BLCKSZ = 8192 SizeOfPageHeaderData = 24 sizeof(IndexTupleData) = 8 sizeof(ItemIdData) = 4 MaxIndexTuplesPerPage is 408. If we have 407 normal and one 0-attr index tuple, they leave 16 byte spare space, in which no normal tuple fits. In the case where we have BLCKSZ of 3 * 8192 = 24576 bytes, MaxIndexTuplesPerPage is 1227. 1226 normal and one 0-attr tuples uses 24556 bytes and it leaves spare 20 bytes, in which one extra normal tuple can reside. This can cause off-by-one error. In reality, all types of index have opaque area with at least 8 bytes long and it prevent pages from having extra tuples. But it doesn't seem to me stable enough. reagards. -- Kyotaro Horiguchi NTT Open Source Software Center
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> At Thu, 12 Apr 2018 17:26:33 -0700, Peter Geoghegan <pg@bowt.ie> wrote in
<CAH2-WzkQmb54Kbx-YHXstRKXcNc+_87jwV3DRb54xcybLR7Oig@mail.gmail.com>
>> However, that at least seems questionable to me. See _bt_pgaddtup()
>> for a simple example of this -- "minus infinity" items on internal
>> pages are sized sizeof(IndexTupleData).
> MaxIndexTuplesPerPage is 408. If we have 407 normal and one
> 0-attr index tuple, they leave 16 byte spare space, in which no
> normal tuple fits. In the case where we have BLCKSZ of 3 * 8192 =
> 24576 bytes, MaxIndexTuplesPerPage is 1227. 1226 normal and one
> 0-attr tuples uses 24556 bytes and it leaves spare 20 bytes, in
> which one extra normal tuple can reside. This can cause
> off-by-one error.
We don't support BLCKSZ that isn't a power of 2, and are unlikely
to start. So that particular counterexample doesn't excite me.
As long as btree only has one no-data tuple per page, I think we are good,
because this calculation does not account for page special space. We might
be underestimating how many tuples can fit by one MAXALIGN quantum, but
the special space takes up at least one MAXALIGN quantum, so it's safe.
Twouldn't be a bad idea to document this reasoning, though.
Also, my first reaction on looking at this code was "who added
MinIndexTupleSize and then didn't replace the equivalent subexpression
of MaxIndexTuplesPerPage with MinIndexTupleSize?". But if
MinIndexTupleSize isn't used anywhere, and I don't see it either,
I think we should just remove the unused macro. It's not buying
anything, and it's confusing because the preceding comment is
about MaxIndexTuplesPerPage and doesn't describe MinIndexTupleSize.
regards, tom lane
On Fri, Apr 13, 2018 at 10:45 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > As long as btree only has one no-data tuple per page, I think we are good, > because this calculation does not account for page special space. We might > be underestimating how many tuples can fit by one MAXALIGN quantum, but > the special space takes up at least one MAXALIGN quantum, so it's safe. > > Twouldn't be a bad idea to document this reasoning, though. Thanks for taking care of this. > Also, my first reaction on looking at this code was "who added > MinIndexTupleSize and then didn't replace the equivalent subexpression > of MaxIndexTuplesPerPage with MinIndexTupleSize?". I had the same reaction. -- Peter Geoghegan