On Sat, Mar 25, 2017 at 11:02 AM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Thu, Mar 23, 2017 at 11:24 PM, Ashutosh Sharma <ashu.coe...@gmail.com> 
> wrote:
>> Hi,
>> On Tue, Feb 7, 2017 at 9:23 AM, Robert Haas <robertmh...@gmail.com> wrote:
>>> On Mon, Feb 6, 2017 at 10:40 PM, Amit Kapila <amit.kapil...@gmail.com> 
>>> wrote:
>>>>> Maybe we should call them "unused pages".
>>>> +1.  If we consider some more names for that column then probably one
>>>> alternative could be "empty pages".
>>> Yeah, but I think "unused" might be better.  Because a page could be
>>> in use (as an overflow page or primary bucket page) and still be
>>> empty.
>> Based on the earlier discussions, I have prepared a patch that would
>> allow pgstathashindex() to show the number of unused pages in hash
>> index. Please find the attached patch for the same. Thanks.
>   else if (opaque->hasho_flag & LH_BITMAP_PAGE)
>   stats.bitmap_pages++;
> + else if (PageIsEmpty(page))
> + stats.unused_pages++;
> I think having this check after PageIsNew() makes more sense then
> having at the place where you currently have,

I don't think having it just below PageIsNew() check would be good.
The reason being, there can be a bucket page which is in use but still
be empty. So, if we add a check just below PageIsNew check then even
though a page is marked as bucket page and is empty it will be shown
as unused page which i feel is not correct. Here is one simple example
that illustrates this point,

select hash_page_type(get_raw_page('con_hash_index', 17));

gdb shows following info for this block number 17,

(gdb) p *(PageHeader)page
$1 = {pd_lsn = {xlogid = 0, xrecoff = 122297104}, pd_checksum = 0,
pd_flags = 0, pd_lower = 24,    pd_upper = 8176, pd_special = 8176,
  pd_pagesize_version = 8196, pd_prune_xid = 0,    pd_linp = 0x22a82d8}

(gdb) p    ((HashPageOpaque) PageGetSpecialPointer(page))->hasho_flag
$2 = 66

(gdb) p    (((HashPageOpaque) PageGetSpecialPointer(page))->hasho_flag
$3 = 2

>From above information it is clear that this page is a bucket page and
is empty. I think it should be shown as bucket page rather than unused
page. Also,  this has already been discussed in [1].

other than that
> code-wise your patch looks okay, although I haven't tested it.

I have tested it properly and didn't find any issues.

> I think this should also be tracked under PostgreSQL 10 open items,
> but as this is not directly a bug, so not sure, what do others think?

Well, Even I am not sure if this has to be added under open items list
or not. Thanks.

[1] - 

With Regards,
Ashutosh Sharma

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to