Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On 22 March 2017 at 03:42, Haribabu Kommi wrote: > > > On Wed, Mar 22, 2017 at 1:53 PM, Seki, Eiji > wrote: >> >> >> Thank you for your review, again. >> >> I think your proposals are better, so I reflected them. > > > Thanks for the updated patch. Patch looks good to me. > I marked it as "ready for committer". Looks good. I'll double check and commit this. > While reviewing this patch, I found that PGXACT->vacuumFlags > variable name needs a rename because with the addition of > PROC_IN_LOGICAL_DECODING flag "vacuumFlags" doesn't > only use it for vacuum operation. I feel this variable can be renamed > as just "flags", but anyway that is a different patch. Good point. Should be an open item. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Wed, Mar 22, 2017 at 1:53 PM, Seki, Eiji wrote: > > Thank you for your review, again. > > I think your proposals are better, so I reflected them. Thanks for the updated patch. Patch looks good to me. I marked it as "ready for committer". While reviewing this patch, I found that PGXACT->vacuumFlags variable name needs a rename because with the addition of PROC_IN_LOGICAL_DECODING flag "vacuumFlags" doesn't only use it for vacuum operation. I feel this variable can be renamed as just "flags", but anyway that is a different patch. Regards, Hari Babu Fujitsu Australia
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On 2017-03-21 07:46:47 Haribabu Kommi wrote: >On Tue, Mar 21, 2017 at 3:16 PM, Seki, Eiji wrote: >+/* Use these flags in GetOldestXmin as "flags" */ > >How about some thing like the following. >/* Use the following flags as an input "flags" to GetOldestXmin function */ > > >+/* Ignore vacuum backends (Note: this also ignores analyze with vacuum >backends) */ >+#define PROCARRAY_FLAGS_VACUUM >PROCARRAY_FLAGS_DEFAULT | PROCARRAY_VACUUM_FLAG >+/* Ignore analyze backends (Note: this also ignores vacuum with analyze >backends) */ >+#define PROCARRAY_FLAGS_ANALYZE >PROCARRAY_FLAGS_DEFAULT | PROCARRAY_ANALYZE_FLAG > >Whenever the above flags are passed to the GetOldestXmin() function, >it just verifies whether any one of the flags are set in the backend flags >or not. And also actually the PROC_IN_ANALYZE flag will set when >analyze operation is started and reset at the end. I feel, it is not >required to mention the Note section. > >+/* Ignore vacuum backends and analyze ones */ > >How about changing it as "Ignore both vacuum and analyze backends". Thank you for your review, again. I think your proposals are better, so I reflected them. -- Regards, Eiji Seki Fujitsu get_oldest_xmin_with_ignore_flags_v4.patch Description: get_oldest_xmin_with_ignore_flags_v4.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Tue, Mar 21, 2017 at 3:16 PM, Seki, Eiji wrote: > > > Thank you for you review. > > I reflected your comment and attach the updated patch. Thanks for the updated patch. +/* Use these flags in GetOldestXmin as "flags" */ How about some thing like the following. /* Use the following flags as an input "flags" to GetOldestXmin function */ +/* Ignore vacuum backends (Note: this also ignores analyze with vacuum backends) */ +#define PROCARRAY_FLAGS_VACUUM PROCARRAY_FLAGS_DEFAULT | PROCARRAY_VACUUM_FLAG +/* Ignore analyze backends (Note: this also ignores vacuum with analyze backends) */ +#define PROCARRAY_FLAGS_ANALYZE PROCARRAY_FLAGS_DEFAULT | PROCARRAY_ANALYZE_FLAG Whenever the above flags are passed to the GetOldestXmin() function, it just verifies whether any one of the flags are set in the backend flags or not. And also actually the PROC_IN_ANALYZE flag will set when analyze operation is started and reset at the end. I feel, it is not required to mention the Note section. +/* Ignore vacuum backends and analyze ones */ How about changing it as "Ignore both vacuum and analyze backends". Regards, Hari Babu Fujitsu Australia
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On 2017-02-24 04:17:20 Haribabu Kommi wrote: >On Fri, Feb 24, 2017 at 3:17 PM, Seki, Eiji > >wrote: > >> >> Thank you for your comments. >> >> I reflected these comments to the attached patch. And I renamed IGNORE_XXX >> flags to PROCARRAY_XXX flags. > > >I checked the latest patch and I have some comments. > >+static int >+ConvertProcarrayFlagToProcFlag(int flags) > >I feel this function is not needed, if we try to maintain same flag values >for both PROC_XXX and PROCARRAY_XXX by writing some comments >in the both the declarations place to make sure that the person modifying >the flag values needs to update them in both the places. I feel it is >usually >rare that the flag values gets changed. > >+ * Now, flags is used only for ignoring backends with some flags. > >How about writing something like below. > >The flags are used to ignore the backends in calculation when any of the >corresponding flags is set. > >+#define PROCARRAY_A_FLAG_VACUUM > >How about changing the flag name to PROCARRAY_VACUUM_FLAG >(similar changes to other declarations) > > >+/* Use these flags in GetOldestXmin as "flags" */ > >Add some comments here to explain how to use these flags. > >+#define PROCARRAY_FLAGS_DEFAULT > >same here also as PROCARRAY_DEFAULT_FLAGS >(similar changes to other declarations) Thank you for you review. I reflected your comment and attach the updated patch. -- Regards, Eiji Seki Fujitsu get_oldest_xmin_with_ignore_flags_v3.patch Description: get_oldest_xmin_with_ignore_flags_v3.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Thu, Mar 16, 2017 at 12:29 AM, Haribabu Kommi wrote: > On Fri, Feb 24, 2017 at 3:17 PM, Seki, Eiji > wrote: >> Thank you for your comments. >> >> I reflected these comments to the attached patch. And I renamed IGNORE_XXX >> flags to PROCARRAY_XXX flags. > > I checked the latest patch and I have some comments. > > +static int > +ConvertProcarrayFlagToProcFlag(int flags) > > I feel this function is not needed, if we try to maintain same flag values > for both PROC_XXX and PROCARRAY_XXX by writing some comments > in the both the declarations place to make sure that the person modifying > the flag values needs to update them in both the places. I feel it is > usually > rare that the flag values gets changed. Yeah, it doesn't seem like a good idea to add additional computation to something that's already a known hot spot. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Fri, Feb 24, 2017 at 3:17 PM, Seki, Eiji wrote: > > Thank you for your comments. > > I reflected these comments to the attached patch. And I renamed IGNORE_XXX > flags to PROCARRAY_XXX flags. I checked the latest patch and I have some comments. +static int +ConvertProcarrayFlagToProcFlag(int flags) I feel this function is not needed, if we try to maintain same flag values for both PROC_XXX and PROCARRAY_XXX by writing some comments in the both the declarations place to make sure that the person modifying the flag values needs to update them in both the places. I feel it is usually rare that the flag values gets changed. + * Now, flags is used only for ignoring backends with some flags. How about writing something like below. The flags are used to ignore the backends in calculation when any of the corresponding flags is set. +#define PROCARRAY_A_FLAG_VACUUM How about changing the flag name to PROCARRAY_VACUUM_FLAG (similar changes to other declarations) +/* Use these flags in GetOldestXmin as "flags" */ Add some comments here to explain how to use these flags. +#define PROCARRAY_FLAGS_DEFAULT same here also as PROCARRAY_DEFAULT_FLAGS (similar changes to other declarations) Regards, Hari Babu Fujitsu Australia
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
on 2017-02-24 04:41:09 Simon Riggs wrote: > ...you didn't comment at all on the accuracy and usefulness of the > gathered statistics, when the sample is biased towards non-updated > data. In my understanding, the sample for statistics is not biased at least in our use case because the conversion process from WOS to ROS never modify (but only read) indexed table and modify only an index relation. And I think such a process that modifies some tables should not ignore analyze processes. -- Regards, Eiji Seki Fujitsu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On 14 February 2017 at 14:19, Seki, Eiji wrote: > Hi all, > > I propose the patch that adds a function GetOldestXminExtended that is like > GetOldestXmin but can ignore arbitrary vacuum flags. And then, rewrite > GetOldestXmin to use it. Note that this is done so as not to change the > behavior of GetOldestXmin. FWIW, I have changes in the logical decoding on standby patch that also need to extend GetOldestXmin. Specifically, I need to be able to return the catalog_xmin separately, rather than the current behaviour of returning what's effectively min(xmin,catalog_xmin). It's only loosely related to this change, but may be relevant. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On 16 February 2017 at 05:24, Seki, Eiji wrote: > Simon Riggs wrote: >> Please persuade us with measurements that allowing this impact on >> ANALYZE would really improve performance at least in your case, and >> also examine the effect of this on the accuracy and usefulness of the >> gathered statistics. > > I explain results of the test that Haribabu mentioned in [1]. Thanks for the tests. I can see the performance is affected by this. > Please let me know if you need any further information. ...you didn't comment at all on the accuracy and usefulness of the gathered statistics, when the sample is biased towards non-updated data. I'm wondering whether this should be an additional table-level option. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On 2017-02-15 17:27:11 Robert Haas wrote: > On Tue, Feb 14, 2017 at 1:38 PM, Jim Nasby > wrote: > > On 2/14/17 3:13 AM, Seki, Eiji wrote: > >> +extern TransactionId GetOldestXmin(Relation rel, uint8 ignoreFlags); > > > > > > My impression is that most other places that do this sort of thing just call > > the argument 'flags', so as not to "lock in" a single idea of what the flags > > are for. I can't readily think of another use for flags in GetOldestXmin, > > but ISTM it's better to just go with "flags" instead of "ignoreFlags". > > I agree; also, many years ago a guy named Tom Lane told me that flags > argument should typically be declared as type "int". I've followed > that advice ever since. Thank you for your comments. I reflected these comments to the attached patch. And I renamed IGNORE_XXX flags to PROCARRAY_XXX flags. -- Regards, Eiji Seki Fujitsu get_oldest_xmin_with_ignore_flags_v2.patch Description: get_oldest_xmin_with_ignore_flags_v2.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
Simon Riggs wrote: > Please persuade us with measurements that allowing this impact on > ANALYZE would really improve performance at least in your case, and > also examine the effect of this on the accuracy and usefulness of the > gathered statistics. I explain results of the test that Haribabu mentioned in [1]. The purpose of this test is the followings. - Check the effect of VCI to OLTP workload - Check whether VCI can be used in OLAP query even if there is OLTP workload at a same table The test is done as the followings. - Use the Tiny TPC-C [2] benchmark as OLTP workload - Scale factor: 100 - Create VCI on the 'stock' table before starting benchmark - Planner doesn't select VCI for queries of the Tiny TPC-C. I attach the result graph. This graph indicates the transition of the number of rows in WOS. In our environment, when the WOS size exceeds about 700,000, VCI is no longer used as such the following query. select count(*) from stock where s_order_cnt > 4; While in low load ("Number of clients = 10" line, the throughput was about 1,000) the WOS size didn't exceed about 500,000, in high load ("Number of clients = 30 (Without patch)" line, the throughput was about 1,400) the WOS size frequently exceeded 700,000. While the WOS size continued to increase, ANALYZE only (without VACUUM) process created by autovacuum daemon always ran and conversion process from WOS to ROS didn't run. Then, after changing to ignore ANALYZE only processes using my patch, the WOS size no longer exceeded about 500,000 ("Number of clients = 30 (With patch)" line, the throughput was about 1,400). Please let me know if you need any further information. [1] - https://www.postgresql.org/message-id/CAJrrPGen1bJYRHu7VFp13QZUyaLdX5N4AH1cqQdiNd8uLVZWow%40mail.gmail.com [2] - http://hp.vector.co.jp/authors/VA052413/jdbcrunner/manual_ja/tpc-c.html (Sorry, there is Japanese document only) -- Regards, Eiji Seki Fujitsu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Thu, Feb 16, 2017 at 10:48 AM, Haribabu Kommi wrote: > Because of the above reason, we need a new API or some change in API > to provide the Oldest xmin by ignoring the ANALYZE transactions, so that > it will reduce the size of WOS and improves the VCI query performance. A new API in core is not completely mandatory either. As mentioned during last week developer meeting to Seki-san, as the list of PGXACT and PGPROC is in shared memory you could as well develop your own version. Still there is a limitation with KnownAssignedXidsGetOldestXmin in recovery, which may be better if exposed at quick glance but you actually don't care for storage, right? If the change makes sense, it seems to me that making a routine more extensible makes the most sense in this case if the API extension can be used by modules with more goals in mind. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Wed, Feb 15, 2017 at 11:35 PM, Amit Kapila wrote: > On Wed, Feb 15, 2017 at 12:03 PM, Seki, Eiji > wrote: > > Amit Kapila wrote: > >> How will you decide just based on oldest xmin whether the tuple is > visible or not? How will you take decisions about tuples which have xmax > set? > > > > In our use case, GetOldestXmin is used by an original maintainer > process[es] to an original control table[s]. The table can be concurrently > read or inserted in any transactions. However, rows in the table can be > deleted (set xmax) only by the maintainer process. Then, one control table > can be processed by only one maintainer process at once. > > > > So I do MVCC as following. > > > > - The maintainer's transaction: > > - If xmax is set, simply ignore the tuple. > > - For other tuples, read tuples if GetOldestXmin() > xmin. > > - Other transactions: Do ordinal MVCC using his XID. > > > > Oh, this is a very specific case for which such an API can be useful. > Earlier, I have seen that people proposing some kind of hooks which > can be used for their specific purpose but exposing an API or changing > the signature of an API sound bit awkward. Consider tomorrow someone > decides to change this API for some reason, it might become difficult > to decide because we can't find it's usage. The proposed change of new API is in the context of fixing the performance problem of vertical clustered index feature [1]. During the performance test of VCI in parallel with OLTP queries, sometimes the query performance is getting dropped because of not choosing the VCI as the best plan. This is due to increase of more records in WOS relation that are needed to be moved to ROS. If there are more records in WOS relation, the it takes more time to generate the LOCAL ROS, because of this reason, the VCI plan is not chosen. Why there are more records in WOS? We used GetOldestXmin() function to identify the minimum transaction that is present in the cluster in order to move the data from WOS to ROS. This function doesn't ignore the ANALYZE transactions that are running in the system. As these transactions doesn't do any changes that will affect the data movement from WOS to ROS. Because of the above reason, we need a new API or some change in API to provide the Oldest xmin by ignoring the ANALYZE transactions, so that it will reduce the size of WOS and improves the VCI query performance. [1] - https://www.postgresql.org/message-id/cajrrpgfac7wc9nk6ptty6yn-nn+hcy8xolah2doyhvg5d6h...@mail.gmail.com Regards, Hari Babu Fujitsu Australia
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
Andres Freund writes: > On 2017-02-15 12:27:11 -0500, Robert Haas wrote: >> I agree; also, many years ago a guy named Tom Lane told me that flags >> argument should typically be declared as type "int". I've followed >> that advice ever since. > Why is that? I think uint makes a lot more sense for flags where the > flags are individual bits that set/unset. Doing that with the sign bit > isn't a good idea. It would only matter if you got to the point of needing the sign bit for a flag, which basically never happens for this sort of usage (or if it does, you'd better be thinking about switching to int64 anyway). So the extra mental and typing load of choosing some other type than plain old "int" isn't really justified, IMO. Anyway the real point is that "int" is preferable to "uint8" which was the original choice here. "uint8" unduly constrains the number of flags you can add without an ABI break, and on many machines it's more efficient to pass "int" function arguments than some other width. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On 2017-02-15 12:27:11 -0500, Robert Haas wrote: > On Tue, Feb 14, 2017 at 1:38 PM, Jim Nasby wrote: > > On 2/14/17 3:13 AM, Seki, Eiji wrote: > >> +extern TransactionId GetOldestXmin(Relation rel, uint8 ignoreFlags); > > > > > > My impression is that most other places that do this sort of thing just call > > the argument 'flags', so as not to "lock in" a single idea of what the flags > > are for. I can't readily think of another use for flags in GetOldestXmin, > > but ISTM it's better to just go with "flags" instead of "ignoreFlags". > > I agree; also, many years ago a guy named Tom Lane told me that flags > argument should typically be declared as type "int". I've followed > that advice ever since. Why is that? I think uint makes a lot more sense for flags where the flags are individual bits that set/unset. Doing that with the sign bit isn't a good idea. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Tue, Feb 14, 2017 at 1:38 PM, Jim Nasby wrote: > On 2/14/17 3:13 AM, Seki, Eiji wrote: >> +extern TransactionId GetOldestXmin(Relation rel, uint8 ignoreFlags); > > > My impression is that most other places that do this sort of thing just call > the argument 'flags', so as not to "lock in" a single idea of what the flags > are for. I can't readily think of another use for flags in GetOldestXmin, > but ISTM it's better to just go with "flags" instead of "ignoreFlags". I agree; also, many years ago a guy named Tom Lane told me that flags argument should typically be declared as type "int". I've followed that advice ever since. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Wed, Feb 15, 2017 at 12:03 PM, Seki, Eiji wrote: > Amit Kapila wrote: >> How will you decide just based on oldest xmin whether the tuple is visible >> or not? How will you take decisions about tuples which have xmax set? > > In our use case, GetOldestXmin is used by an original maintainer process[es] > to an original control table[s]. The table can be concurrently read or > inserted in any transactions. However, rows in the table can be deleted (set > xmax) only by the maintainer process. Then, one control table can be > processed by only one maintainer process at once. > > So I do MVCC as following. > > - The maintainer's transaction: > - If xmax is set, simply ignore the tuple. > - For other tuples, read tuples if GetOldestXmin() > xmin. > - Other transactions: Do ordinal MVCC using his XID. > Oh, this is a very specific case for which such an API can be useful. Earlier, I have seen that people proposing some kind of hooks which can be used for their specific purpose but exposing an API or changing the signature of an API sound bit awkward. Consider tomorrow someone decides to change this API for some reason, it might become difficult to decide because we can't find it's usage. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
Michael Paquier wrote: > On Wed, Feb 15, 2017 at 5:32 PM, Simon Riggs wrote: > > Please persuade us with measurements that allowing this impact on > > ANALYZE would really improve performance at least in your case, and > > also examine the effect of this on the accuracy and usefulness of the > > gathered statistics. > > FWIW, there was last week a developer meeting in Tokyo, and Seki-san > has presented such measurements. So it is not like nothing exists in > this area. Sounds like it'd be a good idea to present these results to this list, too. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Wed, Feb 15, 2017 at 5:32 PM, Simon Riggs wrote: > Please persuade us with measurements that allowing this impact on > ANALYZE would really improve performance at least in your case, and > also examine the effect of this on the accuracy and usefulness of the > gathered statistics. FWIW, there was last week a developer meeting in Tokyo, and Seki-san has presented such measurements. So it is not like nothing exists in this area. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On 14 February 2017 at 06:19, Seki, Eiji wrote: > 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. ... > I'm not sure that this feature is useful in general. > Please let me know your opinion if you are interested. You mention the above problem and hypothesise a solution. IMHO the discussion on this is the wrong way around. First we must be certain that the solution is effective and has no problems, then we decide how to code it or discuss APIs. If you do this, ANALYZE will see an inconsistent view of data in the table, biasing its view towards data not recently updated, which might also have a negative performance effect. Please persuade us with measurements that allowing this impact on ANALYZE would really improve performance at least in your case, and also examine the effect of this on the accuracy and usefulness of the gathered statistics. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
Jim Nasby wrote: > On 2/14/17 3:13 AM, Seki, Eiji wrote: > > +extern TransactionId GetOldestXmin(Relation rel, uint8 > > ignoreFlags); > > My impression is that most other places that do this sort of thing just call > the argument 'flags', so as not to "lock in" a single idea of what the flags > are for. I can't readily think of another use for flags in GetOldestXmin, but > ISTM it's better to just go with "flags" instead of "ignoreFlags". Thanks. I also think "flags" is better. I will rename it. But I wonder if I should rename the defined flag names, IGNORE_A_FLAG_XXX and IGNORE_FLAGS_XXX because they include "IGNORE" in their name. I'm concerned GetOldestXmin users are difficult to know the meaning if they have general names, and general names will conflict to other definitions. Would you let me know if you have any idea? -- Regards, Eiji Seki Fujitsu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
Amit Kapila wrote: > How will you decide just based on oldest xmin whether the tuple is visible or > not? How will you take decisions about tuples which have xmax set? In our use case, GetOldestXmin is used by an original maintainer process[es] to an original control table[s]. The table can be concurrently read or inserted in any transactions. However, rows in the table can be deleted (set xmax) only by the maintainer process. Then, one control table can be processed by only one maintainer process at once. So I do MVCC as following. - The maintainer's transaction: - If xmax is set, simply ignore the tuple. - For other tuples, read tuples if GetOldestXmin() > xmin. - Other transactions: Do ordinal MVCC using his XID. Note: A control table relates to a normal table relation, so get oldest xmin as to the normal table. -- Regards, Eiji Seki Fujitsu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On 2/14/17 3:13 AM, Seki, Eiji wrote: +extern TransactionId GetOldestXmin(Relation rel, uint8 ignoreFlags); My impression is that most other places that do this sort of thing just call the argument 'flags', so as not to "lock in" a single idea of what the flags are for. I can't readily think of another use for flags in GetOldestXmin, but ISTM it's better to just go with "flags" instead of "ignoreFlags". -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Tue, Feb 14, 2017 at 11:49 AM, Seki, Eiji wrote: > Hi all, > > I propose the patch that adds a function GetOldestXminExtended that is like > GetOldestXmin but can ignore arbitrary vacuum flags. And then, rewrite > GetOldestXmin to use it. Note that this is done so as not to change the > behavior of GetOldestXmin. > > 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. > How will you decide just based on oldest xmin whether the tuple is visible or not? How will you take decisions about tuples which have xmax set? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
Michael Paquier 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 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
Re: [HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
On Tue, Feb 14, 2017 at 3:19 PM, Seki, Eiji 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
[HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags
Hi all, I propose the patch that adds a function GetOldestXminExtended that is like GetOldestXmin but can ignore arbitrary vacuum flags. And then, rewrite GetOldestXmin to use it. Note that this is done so as not to change the behavior of GetOldestXmin. 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. GetOldestXminExtended(rel, PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING) This change should have no impact to the existing GetOldestXmin without slight overhead to call the function. I'm not sure that this feature is useful in general. Please let me know your opinion if you are interested. Regards, Eiji Seki Fujitsu. get_oldest_xmin_extended.patch Description: get_oldest_xmin_extended.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers