Re: [PATCH] Unremovable tuple monitoring

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_vacuum which I don't think was particularly good, and I've replace the word "tuple" with "row" in some docs I 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 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.

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 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.
>

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 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.

Fair enough! I'll review this as well.

Thanks again!

--Royce