[HACKERS] Fix typo in blvacuum.c

2017-10-16 Thread Seki, Eiji 
Hi all,

I found a typo in comment of blvacuum.c, so attach the patch for it.

--
Regards,
Eiji Seki
Fujitsu




fix_comment_in_blvacuum_c.patch
Description: fix_comment_in_blvacuum_c.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] BRIN de-summarize ranges

2017-03-22 Thread Seki, Eiji 
On 2017-02-28 04:56:43 Alvaro Herrera wrote:
> Here's a small patch to make a BRIN page range unsummarized.  This is
> useful if data has been deleted, and the heap pages are now used for
> completely different data.

Hi,

I tried to apply your patch and use it. Applying and "make check" were 
successed.

However, I found that when calling brin_desummarize_range successively, an 
assertion is failed. It seems to me that it occurs when desummarizing a revmap 
page that is already desummarized.

The tried queries and their outputs are the followings:

$ CREATE SCHEMA test_sc;
CREATE SCHEMA
$ CREATE TABLE test_sc.test(a int);
CREATE TABLE
$ INSERT INTO test_sc.test SELECT s FROM generate_series(1, 1) s;
INSERT 0 1
$ CREATE INDEX idx_brin ON test_sc.test USING brin(a);
CREATE INDEX
$ SELECT brin_desummarize_range('test_sc.idx_brin', 1);
 brin_desummarize_range 

 
(1 row)

$ SELECT brin_desummarize_range('test_sc.idx_brin', 1);
psql:check_brin_desum.sql:10: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
psql:check_brin_desum.sql:10: connection to server was lost


Then, the server log is the following:

TRAP: FailedAssertion("!(!LWLockHeldByMe(((LWLock*) (&(buf)->content_lock", 
File: "bufmgr.c", Line: 1714)
2017-03-22 15:06:12.842 JST [23107] LOG:  server process (PID 23186) was 
terminated by signal 6: Aborted
2017-03-22 15:06:12.842 JST [23107] DETAIL:  Failed process was running: SELECT 
brin_desummarize_range('test_sc.idx_brin', 1);


When assertion is failed, the following brinRevmapTerminate function is called. 
Then, it tries to release revmap->rm_currBuf by ReleaseBuffer function and it 
is failed.

+bool
+brinRevmapDesummarizeRange(Relation idxrel, BlockNumber heapBlk)
+{
...
+   if (!ItemPointerIsValid(iptr))
+   {
+   /* no index tuple: range not summarized, we're done */
+   brinRevmapTerminate(revmap);
+   return true;
+   }

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

2017-03-21 Thread Seki, Eiji 
On 2017-03-21 07:46:47 Haribabu Kommi wrote:
>On Tue, Mar 21, 2017 at 3:16 PM, Seki, Eiji <seki.e...@jp.fujitsu.com> 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

2017-03-20 Thread Seki, Eiji 
On 2017-02-24 04:17:20 Haribabu Kommi wrote:
>On Fri, Feb 24, 2017 at 3:17 PM, Seki, Eiji 
><seki(dot)eiji(at)jp(dot)fujitsu(dot)com>
>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

[HACKERS] [Doc fix] Wrong explanation about tsquery_phrase

2017-02-28 Thread Seki, Eiji 
Hi all,

I found a wrong explanation about tsquery_phrase. I was slightly confused when 
I tried to use it.

The current document explains tsquery_phrase as the followings[1].

- Function: tsquery_phrase(query1 tsquery, query2 tsquery, distance integer)
- Return Type: tsquery
- Description: make query that searches for query1 followed by query2 at 
maximum distance distance

However, 'exact distance' seems to be right instead of 'maximum distance'.

This was probably overlooked in the following commit.

028350f619f7688e0453fcd2c4b25abe9ba30fa7

[1] -- https://www.postgresql.org/docs/9.6/static/functions-textsearch.html

--
Regards,
Eiji Seki
Fujitsu


fix_document_about_tsquery_phrase.patch
Description: fix_document_about_tsquery_phrase.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

2017-02-27 Thread Seki, Eiji 
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

2017-02-23 Thread Seki, Eiji 
On 2017-02-15 17:27:11  Robert Haas wrote:
> On Tue, Feb 14, 2017 at 1:38 PM, Jim Nasby 
> <Jim(dot)Nasby(at)bluetreble(dot)com> 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

2017-02-15 Thread Seki, Eiji 
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

2017-02-14 Thread Seki, Eiji 
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

2017-02-14 Thread Seki, Eiji 
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

2017-02-14 Thread Seki, Eiji 
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

[HACKERS] Proposal: GetOldestXminExtend for ignoring arbitrary vacuum flags

2017-02-13 Thread Seki, Eiji 
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