Michael Paquier <michael.paqu...@gmail.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-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier Sent: Tuesday, February 14, 2017 3:43 PM To: Seki, Eiji/関 栄二 Cc: pgsql-hackers@postgresql.org Subject: Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags On Tue, Feb 14, 2017 at 3:19 PM, Seki, Eiji <seki.e...@jp.fujitsu.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@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
get_oldest_xmin_with_ignore_flags.patch
Description: get_oldest_xmin_with_ignore_flags.patch
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
