Re: [PATCH] Unremovable tuple monitoring

On 2011-10-05 00:45, Royce Ausburn wrote:
> Attached is v2 of my patch that doesn't update CATALOG_VERSION_NO.  I've also fixed the name of an argument to
pgstat_report_vacuumwhich I don't think was particularly good, and I've replace the word "tuple" with "row" in some
docsI added for consistency.
 
>


Hello Royce,

I reviewed your patch. I think it is in good shape, my two main remarks 
(name of n_unremovable_tup and a remark about documentation at the end 
of this review) are highly subjective and I wouldn't spend time on it 
unless other people have the same opinion.

Remarks:

* rules regression test fails because pg_stat_all_tables is changed, 
pg_stat_sys_tables and pg_stat_user_tables as well, but the new 
expected/rules.out is not part of the patch.

* I'm not sure about the name n_unremovable_tup, since it doesn't convey 
it is about dead tuples and judging from only the name it might as well 
include the live tuples. It also doesn't hint that it is a transient 
condition, which vacuum verbose does with the word 'yet'.
What about n_locked_dead_tup? - this contains both information that it 
is about dead tuples, and the lock suggests that once the lock is 
removed, the dead tuple can be removed.

* The number shows correctly in the pg_stat_relation. This is a testcase 
that gives unremovable dead rows:

A:
create table b (a int);
insert into b values (1);

B:
begin transaction ISOLATION LEVEL repeatable read;
select * from b;

A:
update t set b=2 where i=10;
vacuum verbose t;

Then something similar is shown:

INFO:  vacuuming "public.t"
INFO:  index "t_pkey" now contains 1 row versions in 2 pages
DETAIL:  0 index row versions were removed.
0 index pages have been deleted, 0 are currently reusable.
CPU 0.00s/0.00u sec elapsed 0.00 sec.
INFO:  "t": found 0 removable, 2 nonremovable row versions in 1 out of 1 
pages
DETAIL:  1 dead row versions cannot be removed yet.
There were 0 unused item pointers.
0 pages are entirely empty.
CPU 0.00s/0.01u sec elapsed 0.00 sec.
VACUUM
t=# \x
Expanded display is on.
t=# select * from pg_stat_user_tables;
-[ RECORD 1 ]-----+------------------------------
relid             | 16407
schemaname        | public
relname           | t
seq_scan          | 1
seq_tup_read      | 0
idx_scan          | 1
idx_tup_fetch     | 1
n_tup_ins         | 1
n_tup_upd         | 1
n_tup_del         | 0
n_tup_hot_upd     | 1
n_live_tup        | 2
n_dead_tup        | 0
n_unremovable_tup | 1
last_vacuum       | 2011-11-05 21:15:06.939928+01
last_autovacuum   |
last_analyze      |
last_autoanalyze  |
vacuum_count      | 1
autovacuum_count  | 0
analyze_count     | 0
autoanalyze_count | 0

I did some more tests with larger number of updates which revealed no 
unexpected results.

I was puzzled for a while that n_unremovable_tup >= n_dead_tup doesn't 
hold, after all, the unremovable tuples are dead as well. Neither the 
current documentation nor the documentation added by the patch do help 
in explaining the meaning of n_dead_tup and n_unremovable_tup, which may 
be clear to seasoned vacuum hackers, but not to me. In both the case of 
n_dead_tup it would have been nice if the docs mentioned that dead 
tuples are tuples that are deleted or previous versions of updated 
tuples, and that only analyze updates n_dead_tup (since vacuum cleans 
them), in contrast with n_unremovable_tup that gets updated by vacuum. 
Giving an example of how unremovable dead tuples can be caused would 
IMHO also help understanding.

thanks,
Yeb Havinga