Re: [HACKERS] Intermittent regression test failure from index-only scans patch

2011-10-11 Thread Magnus Hagander
On Sun, Oct 9, 2011 at 06:34, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Oct 8, 2011, at 11:04 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I'm inclined to fix this by changing the test to examine idx_tup_read
 not idx_tup_fetch.  Alternatively, we could have the test force
 enable_indexonlyscan off.  Thoughts?

 No preference.

 I ended up doing it the second way (ie enable_indexonlyscan = off)
 because it turns out that pg_stat_user_tables doesn't have the
 idx_tup_read column --- we track that count per index, not per table.
 I could have complicated the test's stats queries some more, but it
 seemed quite not relevant to the goals of the test.

 Should we have another counter for heap fetches avoided?  Seems like that 
 could be useful to know.

 Hm.  I'm hesitant to add another per-table (or per index?) statistics
 counter because of the resultant bloat in the stats file.  But it
 wouldn't be a bad idea for somebody to take two steps back and rethink
 what we're counting in this area.  The current counter definitions are
 mostly backwards-compatible with pre-8.1 behavior, and it seems like the
 goalposts have moved enough that maybe it's time to break compatibility.

We certainly need *some* way to figure out if this has been used,
IMHO. So yeah, if the current way doesn't scale enough, we need to
think of some other way. But I'm not sure one more counter would
really bloat it that bad? OTOH, repeating that reasoning enough time
will eventually make it enough to care about...

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] Intermittent regression test failure from index-only scans patch

2011-10-11 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Sun, Oct 9, 2011 at 06:34, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Should we have another counter for heap fetches avoided?  Seems like that 
 could be useful to know.

 Hm.  I'm hesitant to add another per-table (or per index?) statistics
 counter because of the resultant bloat in the stats file.

 We certainly need *some* way to figure out if this has been used,
 IMHO. So yeah, if the current way doesn't scale enough, we need to
 think of some other way. But I'm not sure one more counter would
 really bloat it that bad? OTOH, repeating that reasoning enough time
 will eventually make it enough to care about...

You can already tell whether it's happening by comparing idx_tup_read
versus idx_tup_fetch.  Now that measure does conflate some things, like
whether the tuple was not read at all or was read and rejected as not
visible, but I'm not at all convinced that another counter is worth its
weight.  If invisible tuples are a significant part of the table then
index-only scanning isn't going to be very useful to you anyway.

regards, tom lane

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


[HACKERS] Intermittent regression test failure from index-only scans patch

2011-10-08 Thread Tom Lane
I notice that several members of the buildfarm have been consistently
showing the same regression test failure since the index-only scans
patch went in:

*** 
/home/pgbuildfarm/workdir/HEAD/pgsql.27150/src/test/regress/expected/stats.out  
Sat Oct  8 03:20:05 2011
--- 
/home/pgbuildfarm/workdir/HEAD/pgsql.27150/src/test/regress/results/stats.out   
Sat Oct  8 12:30:55 2011
***
*** 94,100 
   WHERE st.relname='tenk2' AND cl.relname='tenk2';
   ?column? | ?column? | ?column? | ?column? 
  --+--+--+--
!  t| t| t| t
  (1 row)
  
  SELECT st.heap_blks_read + st.heap_blks_hit = pr.heap_blks + cl.relpages,
--- 94,100 
   WHERE st.relname='tenk2' AND cl.relname='tenk2';
   ?column? | ?column? | ?column? | ?column? 
  --+--+--+--
!  t| t| t| f
  (1 row)
  
  SELECT st.heap_blks_read + st.heap_blks_hit = pr.heap_blks + cl.relpages,


The diff indicates that the idx_scan count advanced but idx_tup_fetch
did not, which is not so surprising here because tenk2 hasn't been
modified in some time.  If the autovacuum daemon managed to mark it
all-visible before the stats test runs, then an index-only scan will
happen, and bingo, no idx_tup_fetch increment (because indeed no heap
tuple was fetched).

I'm inclined to fix this by changing the test to examine idx_tup_read
not idx_tup_fetch.  Alternatively, we could have the test force
enable_indexonlyscan off.  Thoughts?

regards, tom lane

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


Re: [HACKERS] Intermittent regression test failure from index-only scans patch

2011-10-08 Thread Cédric Villemain
2011/10/8 Tom Lane t...@sss.pgh.pa.us:
 I notice that several members of the buildfarm have been consistently
 showing the same regression test failure since the index-only scans
 patch went in:

 *** 
 /home/pgbuildfarm/workdir/HEAD/pgsql.27150/src/test/regress/expected/stats.out
       Sat Oct  8 03:20:05 2011
 --- 
 /home/pgbuildfarm/workdir/HEAD/pgsql.27150/src/test/regress/results/stats.out 
       Sat Oct  8 12:30:55 2011
 ***
 *** 94,100 
   WHERE st.relname='tenk2' AND cl.relname='tenk2';
   ?column? | ?column? | ?column? | ?column?
  --+--+--+--
 !  t        | t        | t        | t
  (1 row)

  SELECT st.heap_blks_read + st.heap_blks_hit = pr.heap_blks + cl.relpages,
 --- 94,100 
   WHERE st.relname='tenk2' AND cl.relname='tenk2';
   ?column? | ?column? | ?column? | ?column?
  --+--+--+--
 !  t        | t        | t        | f
  (1 row)

  SELECT st.heap_blks_read + st.heap_blks_hit = pr.heap_blks + cl.relpages,


 The diff indicates that the idx_scan count advanced but idx_tup_fetch
 did not, which is not so surprising here because tenk2 hasn't been
 modified in some time.  If the autovacuum daemon managed to mark it
 all-visible before the stats test runs, then an index-only scan will
 happen, and bingo, no idx_tup_fetch increment (because indeed no heap
 tuple was fetched).

 I'm inclined to fix this by changing the test to examine idx_tup_read
 not idx_tup_fetch.  Alternatively, we could have the test force
 enable_indexonlyscan off.  Thoughts?

No preferences, but is it interesting to add a vacuum freeze
somewhere and check expected result after index-only scan ? (for both
idx_tup_read and idx_tup_fetch)


                        regards, tom lane

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




-- 
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

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


Re: [HACKERS] Intermittent regression test failure from index-only scans patch

2011-10-08 Thread Tom Lane
=?ISO-8859-1?Q?C=E9dric_Villemain?= cedric.villemain.deb...@gmail.com writes:
 2011/10/8 Tom Lane t...@sss.pgh.pa.us:
 The diff indicates that the idx_scan count advanced but idx_tup_fetch
 did not, which is not so surprising here because tenk2 hasn't been
 modified in some time.  If the autovacuum daemon managed to mark it
 all-visible before the stats test runs, then an index-only scan will
 happen, and bingo, no idx_tup_fetch increment (because indeed no heap
 tuple was fetched).
 
 I'm inclined to fix this by changing the test to examine idx_tup_read
 not idx_tup_fetch.  Alternatively, we could have the test force
 enable_indexonlyscan off.  Thoughts?

 No preferences, but is it interesting to add a vacuum freeze
 somewhere and check expected result after index-only scan ? (for both
 idx_tup_read and idx_tup_fetch)

This test is only trying to make sure that the stats collection
machinery is working.  I don't think that we should try to coerce things
so that it can check something as context-sensitive as whether an
index-only scan happened.  It's too fragile already --- we've seen
non-reproducible failures here many times before.

regards, tom lane

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


Re: [HACKERS] Intermittent regression test failure from index-only scans patch

2011-10-08 Thread Robert Haas
On Oct 8, 2011, at 11:04 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I'm inclined to fix this by changing the test to examine idx_tup_read
 not idx_tup_fetch.  Alternatively, we could have the test force
 enable_indexonlyscan off.  Thoughts?

No preference.

Should we have another counter for heap fetches avoided?  Seems like that could 
be useful to know.

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


Re: [HACKERS] Intermittent regression test failure from index-only scans patch

2011-10-08 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Oct 8, 2011, at 11:04 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I'm inclined to fix this by changing the test to examine idx_tup_read
 not idx_tup_fetch.  Alternatively, we could have the test force
 enable_indexonlyscan off.  Thoughts?

 No preference.

I ended up doing it the second way (ie enable_indexonlyscan = off)
because it turns out that pg_stat_user_tables doesn't have the
idx_tup_read column --- we track that count per index, not per table.
I could have complicated the test's stats queries some more, but it
seemed quite not relevant to the goals of the test.

 Should we have another counter for heap fetches avoided?  Seems like that 
 could be useful to know.

Hm.  I'm hesitant to add another per-table (or per index?) statistics
counter because of the resultant bloat in the stats file.  But it
wouldn't be a bad idea for somebody to take two steps back and rethink
what we're counting in this area.  The current counter definitions are
mostly backwards-compatible with pre-8.1 behavior, and it seems like the
goalposts have moved enough that maybe it's time to break compatibility.

regards, tom lane

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