Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitraryvacuum flags

Michael Paquier <michael.paquier@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
andget 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@postgresql.org [mailto:pgsql-hackers-owner@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.eiji@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
specificprocesses (VACUUM, ANALYZE, etc...) for performance. Our company (Fujitsu) is developing such an extension. In
ourbenchmark, we found that waiting an ANALYZE process created by autovacuum daemon often has a significant impact to
theperformance although the waited process do only reading as to the table. So I hope to ignore it using
GetOldestXminExtendas 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
getthose 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

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