On 16/11/2011, at 2:05 AM, Yeb Havinga wrote:
> 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.
>>
>
> I reviewed your patch. I think it is in good shape, my two main remarks (name of n_unremovable_tup and a remark about
documentationat the end of this review) are highly subjective and I wouldn't spend time on it unless other people have
thesame 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.
Doh! Thank you for spotting this. Should we decide to continue this patch I'll look in to fixing this.
>
> * I'm not sure about the name n_unremovable_tup, since it doesn't convey it is about dead tuples and judging from
onlythe name it might as well include the live tuples. It also doesn't hint that it is a transient condition, which
vacuumverbose 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
thatonce the lock is removed, the dead tuple can be removed.
>
Looks like we have some decent candidates later in the thread. Should this patch survive I'll look at updating it.
> * The number shows correctly in the pg_stat_relation. This is a testcase that gives unremovable dead rows:
>
> I was puzzled for a while that n_unremovable_tup >= n_dead_tup doesn't hold, after all, the unremovable tuples are
deadas well. Neither the current documentation nor the documentation added by the patch do help in explaining the
meaningof n_dead_tup and n_unremovable_tup, which may be clear to seasoned vacuum hackers, but not to me. In both the
caseof n_dead_tup it would have been nice if the docs mentioned that dead tuples are tuples that are deleted or
previousversions of updated tuples, and that only analyze updates n_dead_tup (since vacuum cleans them), in contrast
withn_unremovable_tup that gets updated by vacuum. Giving an example of how unremovable dead tuples can be caused would
IMHOalso help understanding.
Fair enough! I'll review this as well.
Thanks again!
--Royce
