Re: Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags

Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
> It seems to me that it would be far less confusing to just replace the boolean argument of GetOldestXmin by a uint8 and get those flags declared in procarray.h, no? There are a couple of code path calling
> GetOldestXmin() that do not include proc.h, so it would be better to not add any extra header dependencies in those files.

Thank you for your feedback.

Yes, I agree that such extra dependencies are not desirable. I understood that such as the following implementation is better (I attach a patch for reference). Please let me know if my understanding is not correct.

1. Change the arguments of GetOldestXmin
-extern TransactionId GetOldestXmin(Relation rel, bool ignoreVacuum);
+extern TransactionId GetOldestXmin(Relation rel, uint8 ignoreFlags);

2. Add IGNORE_FLAGS_XXX for above "ignoreFlags" in procarray.h
These flags are converted to combined PROC_XXX flag in procarray.c before ignoring

3. Fix callers of GetOldestXmin
GetOldestXmin(NULL, true) -> GetOldestXmin(NULL, IGNORE_FLAGS_VACUUM)
GetOldestXmin(NULL, false) -> GetOldestXmin(NULL, IGNORE_FLAGS_DEFAULT)

Regards,
Eiji Seki
Fujitsu.

-----Original Message-----
From: pgsql-hackers-owner(at)postgresql(dot)org [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Michael Paquier
Sent: Tuesday, February 14, 2017 3:43 PM
To: Seki, Eiji/関 栄二
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags

On Tue, Feb 14, 2017 at 3:19 PM, Seki, Eiji <seki(dot)eiji(at)jp(dot)fujitsu(dot)com> wrote:
> This change will be useful for features that only reads rows that are visible by all transactions and could not wait specific processes (VACUUM, ANALYZE, etc...) for performance. Our company (Fujitsu) is developing such an extension. In our benchmark, we found that waiting an ANALYZE process created by autovacuum daemon often has a significant impact to the performance although the waited process do only reading as to the table. So I hope to ignore it using GetOldestXminExtend as below. The second argument represents flags to ignore.
>
> GetOldestXminExtended(rel, PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING
> | PROC_IN_ANALYZE);
>
> Note: We can ignore VACUUM processes or VACUUM with ANALYZE processes using the second option of GetOldesXmin, "ignoreVacuum". However, we cannot ignore only ANALYZE processes because the "ignoreVacuum" = true is same to the following.

GetOldestXmin(Relation rel, bool ignoreVacuum) {
+ uint8 ignoreFlags = PROC_IN_LOGICAL_DECODING;
+
+ if (ignoreVacuum)
+ ignoreFlags |= PROC_IN_VACUUM;
+
+ return GetOldestXminExtended(rel, ignoreFlags); }

It seems to me that it would be far less confusing to just replace the boolean argument of GetOldestXmin by a uint8 and get those flags declared in procarray.h, no? There are a couple of code path calling
GetOldestXmin() that do not include proc.h, so it would be better to not add any extra header dependencies in those files.
--
Michael

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