Re: [HACKERS] Move PinBuffer and UnpinBuffer to atomics

[Alexander Korotkov]

On Sun, Apr 10, 2016 at 7:26 AM, Amit Kapila 
wrote:

> On Sun, Apr 10, 2016 at 1:13 AM, Andres Freund  wrote:
>
>> On 2016-04-09 22:38:31 +0300, Alexander Korotkov wrote:
>> > There are results with 5364b357 reverted.
>>
>>
> What exactly is this test?
> I think assuming it is a read-only -M prepared pgbench run where data fits
> in shared buffers.  However if you can share exact details, then I can try
> the similar test.
>

Yes, the test is:

pgbench -s 1000 -c $clients -j 100 -M prepared -S -T 300
(shared_buffers=24GB)


>
>> Crazy that this has such a negative impact. Amit, can you reproduce
>> that?
>
>
> I will try it.
>

Good.


>
>
>> Alexander, I guess for r/w workload 5364b357 is a benefit on that
>> machine as well?
>>
>
> I also think so. Alexander, if try read-write workload with unlogged
> tables, then we should see an improvement.
>

I'll try read-write workload.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] Move PinBuffer and UnpinBuffer to atomics

[Alexander Korotkov]

On Sat, Apr 9, 2016 at 10:49 PM, Andres Freund  wrote:

>
>
> On April 9, 2016 12:43:03 PM PDT, Andres Freund 
> wrote:
> >On 2016-04-09 22:38:31 +0300, Alexander Korotkov wrote:
> >> There are results with 5364b357 reverted.
> >
> >Crazy that this has such a negative impact. Amit, can you reproduce
> >that? Alexander, I guess for r/w workload 5364b357 is a benefit on that
> >machine as well?
>
> How sure are you about these measurements?


I'm pretty sure.  I've retried it multiple times by hand before re-run the
script.


> Because there really shouldn't be clog lookups one a steady state is
> reached...
>

Hm... I'm also surprised. There shouldn't be clog lookups once hint bits
are set.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

[HACKERS] tab completion for alter extension

[Jeff Janes]

tab completion for "alter extension foobar update" yields a list of
tables.  That is actively misleading.  (This is not new in 9.6.)

It should complete to nothing, or "TO" followed by a list of available versions.

The attached patch takes approach 2.  I thought of adding a ";" to the
list completions to signify you can optionally end the command
immediately after UPDATE and have it be a complete command.  But I
thought perhaps that was too clever, and unprecedented.

Will add to commitfest-next

Cheers,

Jeff
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index a62ffe6..85f7a1c
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*** static const SchemaQuery Query_for_list_
*** 820,825 
--- 820,832 
  "  WHERE (%d = pg_catalog.length('%s'))"\
  "AND pg_catalog.quote_ident(name)='%s'"
  
+ /* the silly-looking length condition is just to eat up the current word */
+ #define Query_for_list_of_available_extension_versions_with_TO \
+ " SELECT 'TO ' || pg_catalog.quote_ident(version) "\
+ "   FROM pg_catalog.pg_available_extension_versions "\
+ "  WHERE (%d = pg_catalog.length('%s'))"\
+ "AND pg_catalog.quote_ident(name)='%s'"
+ 
  #define Query_for_list_of_prepared_statements \
  " SELECT pg_catalog.quote_ident(name) "\
  "   FROM pg_catalog.pg_prepared_statements "\
*** psql_completion(const char *text, int st
*** 1414,1419 
--- 1421,1440 
  	else if (Matches3("ALTER", "EXTENSION", MatchAny))
  		COMPLETE_WITH_LIST4("ADD", "DROP", "UPDATE", "SET SCHEMA");
  
+ 	/* ALTER EXTENSION  UPDATE */
+ 	else if (Matches4("ALTER", "EXTENSION", MatchAny, "UPDATE"))
+ 	{
+ 		completion_info_charp = prev2_wd;
+ 		COMPLETE_WITH_QUERY(Query_for_list_of_available_extension_versions_with_TO);
+ 	}
+ 
+ 	/* ALTER EXTENSION  UPDATE TO*/
+ 	else if (Matches5("ALTER", "EXTENSION", MatchAny, "UPDATE", "TO"))
+ 	{
+ 		completion_info_charp = prev3_wd;
+ 		COMPLETE_WITH_QUERY(Query_for_list_of_available_extension_versions);
+ 	}
+ 
  	/* ALTER FOREIGN */
  	else if (Matches2("ALTER", "FOREIGN"))
  		COMPLETE_WITH_LIST2("DATA WRAPPER", "TABLE");

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

[HACKERS] Some other things about contrib/bloom and generic_xlog.c

[Tom Lane]

1. It doesn't seem like generic_xlog.c has thought very carefully about
the semantics of the "hole" between pd_lower and pd_upper.  The mainline
XLOG code goes to some lengths to ensure that the hole stays all-zeroes;
for example RestoreBlockImage() explicitly zeroes the hole when restoring
from a full-page image that has a hole.  But generic_xlog.c's redo routine
does not do anything comparable, nor does GenericXLogFinish make any
effort to ensure that the "hole" is all-zeroes after normal application of
a generic update.  The reason this is of interest is that it means the
contents of the "hole" could diverge between master and slave, or differ
between the original state of a database and what it is after a crash and
recovery.  That would at least complicate forensic comparisons of pages,
and I think it might also break checksumming.  We thought that this was
important enough to take the trouble of explicitly zeroing holes during
mainline XLOG replay.  Shouldn't generic_xlog.c take the same trouble?

2. Unless I'm missing something, contrib/bloom is making no effort
to ensure that bloom index pages can be distinguished from other pages
by pg_filedump.  That's fine if your expectation is that bloom will always
be a toy with no use in production; but otherwise, not so much.  You need
to make sure that the last two bytes of the page special space contain a
uniquely identifiable code; compare spgist_page_id in SPGiST indexes.

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] Move PinBuffer and UnpinBuffer to atomics

[Amit Kapila]

On Sun, Apr 10, 2016 at 1:13 AM, Andres Freund  wrote:

> On 2016-04-09 22:38:31 +0300, Alexander Korotkov wrote:
> > There are results with 5364b357 reverted.
>
>
What exactly is this test?
I think assuming it is a read-only -M prepared pgbench run where data fits
in shared buffers.  However if you can share exact details, then I can try
the similar test.


> Crazy that this has such a negative impact. Amit, can you reproduce
> that?


I will try it.


> Alexander, I guess for r/w workload 5364b357 is a benefit on that
> machine as well?
>

I also think so. Alexander, if try read-write workload with unlogged
tables, then we should see an improvement.



With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Performance improvement for joins where outer side is unique

[David Rowley]

On 8 April 2016 at 11:59, Tom Lane  wrote:
> I did some performance testing on the attached somewhat-cleaned-up patch,
> and convinced myself that the planning time penalty is fairly minimal:
> on the order of a couple percent in simple one-join queries, and less
> than that in very large queries.  Oddly, it seems that the result cacheing
> done in get_optimal_jointype() is only barely worth the trouble in typical
> cases; though if you get a query large enough to require GEQO, it's a win
> because successive GEQO attempts can re-use cache entries made by earlier
> attempts.  (This may indicate something wrong with my testing procedure?
> Seems like diking out the cache should have made more difference.)

It'll really depend on whether you're testing a positive case or a
negative one, since a GEQO join search will be the only time the
non-unique cache is filled, then if you're testing the negative case
then this is the only time you'll get any non-unique cache benefits.
On the other hand, if you were testing with a positive case then I
guess the unique index check is cheaper than we thought. Maybe adding
a couple of other unique indexes before defining the one which proves
the join unique (so that they've got a lower OID) would be enough to
start showing the cache benefits of the unique rel cache.

> I did find by measurement that the negative-cache-entry code produces
> exactly zero hits unless you're in GEQO mode, which is not really
> surprising given the order in which the join search occurs.  So in the
> attached patch I made the code not bother with making negative cache
> entries unless using GEQO, to hopefully save a few nanoseconds.

Yeah, I also noticed this in my testing, and it makes sense given how
the standard join search works. Thanks for making that improvement,
I've not yet looked at the code, but it sounds like a good idea.

> I rebased over f338dd758, did a little bit of code cleanup and fixed some
> bugs in the uniqueness detection logic, but have not reviewed the rest of
> the patch since it's likely all gonna change if we reconsider the JoinType
> representation.
>
> Anyway, I think it would be reasonable to give this patch a few more
> days in view of David's being away through the weekend.  But the RMT
> has final say on that.

Thanks for making the updates and committing the refactor of
analyzejoins.c. The RMT have ruled no extension will be given, so I'll
get this into shape for 9.7 CF1.


-- 
 David Rowley   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] Performance improvement for joins where outer side is unique

[David Rowley]

On 8 April 2016 at 02:46, Tom Lane  wrote:
> I'm also a bit suspicious of the fact that some of the plans in
> aggregates.out changed from merge to hash joins; with basically
> no stats at hand in those tests, that seems dubious.  A quick look
> at what the patch touched in costsize.c suggests that this might
> be because you've effectively allowed cost_hashjoin to give a cost
> discount for inner unique, but provided no similar intelligence
> in cost_mergejoin.

(catching up)

The only possible reason that the merge join plans have become a hash
join is that hash join is costed more cheaply (same as SEMI JOIN) when
the inner side is unique. The reason I didn't cost merge join
differently is that there seems to be no special costing done there
for SEMI joins already. I might be wrong, but I didn't feel like it
was up to this patch to introduce that, though, perhaps a patch should
go in before this one to do that.

I understand this is 9.7 stuff now, but I feel like I should tie up
the lose ends before they get forgotten.

-- 
 David Rowley   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] [COMMITTERS] pgsql: Bloom index contrib module

[Tom Lane]

I wrote:
> I was depressed, though not entirely surprised, to find that you get
> exactly that same line-count coverage if the table size is cut back
> to ONE row.

Oh, I found the flaw in my testing: there are two INSERTs in the test
script and I was changing only one of them.  After correcting that,
the results behave a little more sanely:

   Line Coverage   Functions
1 row: 70.4 %   349 / 496   93.1 %  27 / 29
10 row:73.6 %   365 / 496   93.1 %  27 / 29
100 rows:  73.6 %   365 / 496   93.1 %  27 / 29
1000 rows: 75.4 %   374 / 496   93.1 %  27 / 29

Still, we've reached the most coverage this test can give us at 1000
rows, which still means it's wasting the last 99% of its runtime.

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] [COMMITTERS] pgsql: Bloom index contrib module

[Tom Lane]

Noah Misch  writes:
> On Sat, Apr 09, 2016 at 11:50:08AM -0400, Tom Lane wrote:
>> Would it be possible to dial down the amount of runtime consumed by
>> the regression tests for this module?

> I find this added test duration reasonable.  If someone identifies a way to
> realize similar coverage with lower duration, I'd value that contribution.  -1
> for meeting some runtime target at the expense of coverage.  Older modules
> have rather little test coverage, so they're poor as benchmarks.

That argument is fine in principle, but the thing about applications such
as SQL databases is that shoving more rows through them doesn't in itself
increase test coverage; it may just iterate the loops more times.

The contrib/bloom regression test currently creates a 10-row table.
I wondered how many rows were really required to achieve the same level
of code coverage.  I experimented with gcov, and what I find is that
as of HEAD that test provides these levels of coverage:

 Line Coverage   Functions

contrib/bloom/:  75.4 % 374 / 496   93.1 %  27 / 29
generic_xlog.c:  68.5 % 98 / 14377.8 %  7 / 9

(I looked specifically at generic_xlog.c because the main point of this
contrib module, so far as the core code is concerned, is to exercise
that file.)

I was depressed, though not entirely surprised, to find that you get
exactly that same line-count coverage if the table size is cut back
to ONE row.  Only when you remove the INSERT command entirely is there
any change in these gcov statistics.  Therefore, the last 9 rows
are wasting my time, and yours, and that of every other developer who
will ever run this test suite in future.  I do not like having the
community's time wasted.

I'm all for improving our test coverage, but just shoving lots of rows
through a not-very-well-designed-in-the-first-place regression test
isn't a reliable way to do that.

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] [COMMITTERS] pgsql: Bloom index contrib module

[Peter Geoghegan]

On Sat, Apr 9, 2016 at 4:43 PM, Noah Misch  wrote:
> I find this added test duration reasonable.  If someone identifies a way to
> realize similar coverage with lower duration, I'd value that contribution.  -1
> for meeting some runtime target at the expense of coverage.  Older modules
> have rather little test coverage, so they're poor as benchmarks.

+1.

-- 
Peter Geoghegan


-- 
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] [COMMITTERS] pgsql: Bloom index contrib module

[Noah Misch]

On Sat, Apr 09, 2016 at 11:50:08AM -0400, Tom Lane wrote:
> Teodor Sigaev  writes:
> > Bloom index contrib module
> 
> Would it be possible to dial down the amount of runtime consumed by
> the regression tests for this module?
> 
> On my primary dev machine, "make installcheck" in contrib/bloom takes
> 4.5 seconds, which seems a bit excessive when make installcheck across
> all the rest of contrib takes 22 seconds.
> 
> On prairiedog, which admittedly is one of the slower buildfarm animals
> (though far from the slowest), the contrib-install-check-C step went
> from 2:54 immediately before this patch went in to 4:28 immediately
> after.
> 
> That seems out of line for a single contrib module, especially one of
> unproven usefulness.

I find this added test duration reasonable.  If someone identifies a way to
realize similar coverage with lower duration, I'd value that contribution.  -1
for meeting some runtime target at the expense of coverage.  Older modules
have rather little test coverage, so they're poor as benchmarks.

A feature's lack of usefulness can be a good reason to exclude it from the
tree entirely, but it's a bad reason to slash the feature's test cases.  (I
have not evaluated the usefulness of this access method.)


-- 
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: PL/Pythonu - function ereport

[Tom Lane]

Pavel Stehule  writes:
> here is patch

Pushed, thanks.

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: PL/Pythonu - function ereport

[Pavel Stehule]

Hi

2016-04-08 20:54 GMT+02:00 Pavel Stehule :

>
>
> 2016-04-08 20:52 GMT+02:00 Tom Lane :
>
>> Pavel Stehule  writes:
>> > 2016-04-08 17:38 GMT+02:00 Teodor Sigaev :
>> >> thank you, pushed. Pls, pay attention to buildfarm.
>>
>> > Thank you very much for commit.
>>
>> According to buildfarm member prairiedog, there's a problem in one
>> of the test cases.  I suspect that it's using syntax that doesn't
>> exist in Python 2.3, but don't know enough Python to fix it.
>>
>> Please submit a correction -- we are not moving the goalposts on
>> Python version compatibility for the convenience of one test case.
>>
>
> I'll fix it.
>
>
here is patch

I found related discussion -
http://postgresql.nabble.com/Minimum-supported-version-of-Python-td5796175.html

Regards

Pavel


> Regards
>
> Pavel
>
>>
>> regards, tom lane
>>
>
>
diff --git a/src/pl/plpython/expected/plpython_test.out b/src/pl/plpython/expected/plpython_test.out
new file mode 100644
index 05caba1..bc7707d
*** a/src/pl/plpython/expected/plpython_test.out
--- b/src/pl/plpython/expected/plpython_test.out
*** RETURNS void AS $$
*** 150,171 
  kwargs = { "message":_message, "detail":_detail, "hint":_hint,
  			"sqlstate":_sqlstate, "schema":_schema, "table":_table,
  			"column":_column, "datatype":_datatype, "constraint":_constraint }
! # ignore None values
! plpy.error(**dict((k, v) for k, v in iter(kwargs.items()) if v))
  $$ LANGUAGE plpythonu;
  SELECT raise_exception('hello', 'world');
  ERROR:  plpy.Error: hello
  DETAIL:  world
  CONTEXT:  Traceback (most recent call last):
!   PL/Python function "raise_exception", line 6, in 
! plpy.error(**dict((k, v) for k, v in iter(kwargs.items()) if v))
  PL/Python function "raise_exception"
  SELECT raise_exception('message text', 'detail text', _sqlstate => 'YY333');
  ERROR:  plpy.Error: message text
  DETAIL:  detail text
  CONTEXT:  Traceback (most recent call last):
!   PL/Python function "raise_exception", line 6, in 
! plpy.error(**dict((k, v) for k, v in iter(kwargs.items()) if v))
  PL/Python function "raise_exception"
  SELECT raise_exception(_message => 'message text',
  		_detail => 'detail text',
--- 150,175 
  kwargs = { "message":_message, "detail":_detail, "hint":_hint,
  			"sqlstate":_sqlstate, "schema":_schema, "table":_table,
  			"column":_column, "datatype":_datatype, "constraint":_constraint }
! # ignore None values - should to work on Python2.3
! dict = {}
! for k in kwargs:
! 	if kwargs[k] is not None:
! 		dict[k] = kwargs[k]
! plpy.error(**dict)
  $$ LANGUAGE plpythonu;
  SELECT raise_exception('hello', 'world');
  ERROR:  plpy.Error: hello
  DETAIL:  world
  CONTEXT:  Traceback (most recent call last):
!   PL/Python function "raise_exception", line 10, in 
! plpy.error(**dict)
  PL/Python function "raise_exception"
  SELECT raise_exception('message text', 'detail text', _sqlstate => 'YY333');
  ERROR:  plpy.Error: message text
  DETAIL:  detail text
  CONTEXT:  Traceback (most recent call last):
!   PL/Python function "raise_exception", line 10, in 
! plpy.error(**dict)
  PL/Python function "raise_exception"
  SELECT raise_exception(_message => 'message text',
  		_detail => 'detail text',
*** ERROR:  plpy.Error: message text
*** 180,187 
  DETAIL:  detail text
  HINT:  hint text
  CONTEXT:  Traceback (most recent call last):
!   PL/Python function "raise_exception", line 6, in 
! plpy.error(**dict((k, v) for k, v in iter(kwargs.items()) if v))
  PL/Python function "raise_exception"
  SELECT raise_exception(_message => 'message text',
  		_hint => 'hint text',
--- 184,191 
  DETAIL:  detail text
  HINT:  hint text
  CONTEXT:  Traceback (most recent call last):
!   PL/Python function "raise_exception", line 10, in 
! plpy.error(**dict)
  PL/Python function "raise_exception"
  SELECT raise_exception(_message => 'message text',
  		_hint => 'hint text',
*** SELECT raise_exception(_message => 'mess
*** 191,198 
  ERROR:  plpy.Error: message text
  HINT:  hint text
  CONTEXT:  Traceback (most recent call last):
!   PL/Python function "raise_exception", line 6, in 
! plpy.error(**dict((k, v) for k, v in iter(kwargs.items()) if v))
  PL/Python function "raise_exception"
  DO $$
  DECLARE
--- 195,202 
  ERROR:  plpy.Error: message text
  HINT:  hint text
  CONTEXT:  Traceback (most recent call last):
!   PL/Python function "raise_exception", line 10, in 
! plpy.error(**dict)
  PL/Python function "raise_exception"
  DO $$
  DECLARE
diff --git a/src/pl/plpython/sql/plpython_test.sql b/src/pl/plpython/sql/plpython_test.sql
new file mode 100644
index 6e5b535..294f2dd
*** a/src/pl/plpython/sql/plpython_test.sql
--- b/src/pl/plpython/sql/plpython_test.sql
*** RETURNS void AS $$
*** 102,109 
  kwargs = { "message":_message, "detail":_detail, "hint":_hint,
  			

[HACKERS] Sort push down through a nested loop, for 9.7

[Jeff Janes]

I was testing to see if the newer changes in 9.6 fixed some planning
issues I've seen in prior versions.  It does not, for the ones of
interest to me, but while looking into it I see what seems to be a
missing optimization (in all versions).

If the first child of a nested loop produces naturally ordered output
(from an index), the planner is smart enough to realize that this
ordering passes up and applies to the entire nested loop node:

explain select * from pgbench_accounts a join pgbench_accounts b using
(bid) order by a.aid;
  QUERY PLAN
---
 Nested Loop  (cost=0.29..150007137.29 rows=100 width=194)
   Join Filter: (a.bid = b.bid)
   ->  Index Scan using pgbench_accounts_pkey on pgbench_accounts a
(cost=0.29..4247.29 rows=10 width=97)
   ->  Materialize  (cost=0.00..3140.00 rows=10 width=97)
 ->  Seq Scan on pgbench_accounts b  (cost=0.00..2640.00
rows=10 width=97)

But if a naturally ordering index is not available, it is not smart
enough to push the sort down through the nested loop to the first
child:

set enable_hashjoin TO off;

explain select * from pgbench_accounts a join pgbench_accounts b using
(bid) order by a.filler;
 QUERY PLAN
-
 Sort  (cost=5707453952.44..5732453952.44 rows=100 width=275)
   Sort Key: a.filler
   ->  Nested Loop  (cost=0.00..150005530.00 rows=100 width=275)
 Join Filter: (a.bid = b.bid)
 ->  Seq Scan on pgbench_accounts a  (cost=0.00..2640.00
rows=10 width=97)
 ->  Materialize  (cost=0.00..3140.00 rows=10 width=97)
   ->  Seq Scan on pgbench_accounts b  (cost=0.00..2640.00
rows=10 width=97)


Sorting 10 rows would be a lot faster than sorting 100 rows.

Is this one of the cases where the CPU cycles needed to detect the
opportunity during planning are just not worthwhile, considering how
rarely the opportunity arises?

Cheers,

Jeff


-- 
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] Move PinBuffer and UnpinBuffer to atomics

[Andres Freund]

On April 9, 2016 12:43:03 PM PDT, Andres Freund  wrote:
>On 2016-04-09 22:38:31 +0300, Alexander Korotkov wrote:
>> There are results with 5364b357 reverted.
>
>Crazy that this has such a negative impact. Amit, can you reproduce
>that? Alexander, I guess for r/w workload 5364b357 is a benefit on that
>machine as well?

How sure are you about these measurements? Because there really shouldn't be 
clog lookups one a steady state is reached...

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


-- 
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] Move PinBuffer and UnpinBuffer to atomics

[Andres Freund]

On 2016-04-09 22:38:31 +0300, Alexander Korotkov wrote:
> There are results with 5364b357 reverted.

Crazy that this has such a negative impact. Amit, can you reproduce
that? Alexander, I guess for r/w workload 5364b357 is a benefit on that
machine as well?


> It's much closer to what we had before.

I'm going to apply this later then. If there's some micro optimization
for large x86, we can look into that later.

Greetings,

Andres Freund


-- 
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] Move PinBuffer and UnpinBuffer to atomics

[Alexander Korotkov]

On Sat, Apr 9, 2016 at 11:24 AM, Alexander Korotkov <
a.korot...@postgrespro.ru> wrote:

> On Fri, Apr 8, 2016 at 10:19 PM, Alexander Korotkov <
> a.korot...@postgrespro.ru> wrote:
>
>> On Fri, Apr 8, 2016 at 7:39 PM, Andres Freund  wrote:
>>
>>> As you can see in
>>>
>>
>>> http://archives.postgresql.org/message-id/CA%2BTgmoaeRbN%3DZ4oWENLvgGLeHEvGZ_S_Z3KGrdScyKiSvNt3oA%40mail.gmail.com
>>> I'm planning to apply this sometime this weekend, after running some
>>> tests and going over the patch again.
>>>
>>> Any chance you could have a look over this?
>>>
>>
>> I took a look at this.  Changes you made look good for me.
>> I also run test on 4x18 Intel server.
>>
>
> On the top of current master results are following:
>
> clientsTPS
> 1 12562
> 2 25604
> 4 52661
> 8103209
> 10   128599
> 20   256872
> 30   365718
> 40   432749
> 50   513528
> 60   684943
> 70   696050
> 80   923350
> 90  1119776
> 100 1208027
> 110 1229429
> 120 1163356
> 130 1107924
> 140 1084344
> 150 1014064
> 160  961730
> 170  980743
> 180  968419
>
> The results are quite discouraging because previously we had about 1.5M
> TPS in the peak while we have only about 1.2M now.  I found that it's not
> related to the changes you made in the patch, but it's related to 5364b357
> "Increase maximum number of clog buffers".  I'm making same benchmark with
> 5364b357 reverted.
>

There are results with 5364b357 reverted.

clientsTPS
1  12980
2  27105
4  51969
8 105507
   10 132811
   20 256888
   30 368573
   40 467605
   50 544231
   60 590898
   70 799094
   80 967569
   901211662
  1001352427
  1101432561
  1201480324
  1301486624
  1401492092
  1501461681
  1601426733
  1701409081
  1801366199

It's much closer to what we had before.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

[HACKERS] GenericXLogUnregister seems like a pretty horrid idea

[Tom Lane]

I've been doing some code review in generic_xlog.c, and I find that
GenericXLogUnregister is implemented in what seems to me to be a
completely unsafe way: it shoves over the per-page array entries for
following pages.  The problem with that is that it invalidates the Page
pointers previously returned for those pages by GenericXLogRegister;
an effect that is neither documented, nor reasonable for callers to
cope with.

I cannot see any very good reason why callers should need an unregister
function anyway: if they're doing something so complicated that they might
change their minds partway through an XLOG record through about whether a
page needs to be modified, that code is badly in need of redesign.  (And
yes, I am asserting that against blinsert() in particular.  That's not
nice code.)

I think we should get rid of this function.  If people need to do
something like that, they should do GenericXLogAbort() and start over.

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] multivariate statistics v14

[Tatsuo Ishii]

> But I still think it wouldn't move the patch any closer to committable
> state, because what it really needs is review whether the catalog
> definition makes sense, whether it should be more like pg_statistic,
> and so on. Only then it makes sense to describe the catalog structure
> in the SGML docs, I think. That's why I added some basic SGML docs for
> CREATE/DROP/ALTER STATISTICS, which I expect to be rather stable, and
> not the catalog and other low-level stuff (which is commented heavily
> in the code anyway).

Without "user-level docs" (now I understand that the term means all
SGML docs for you), it is very hard to find a visible
characteristics/behavior of the patch. CREATE/DROP/ALTER STATISTICS
just defines a user interface, and does not help how it affects to the
planning. The READMEs do not help either.

In this case reviewing your code is something like reviewing a program
which has no specification.

That's the reason why I said before below, but it was never seriously
considered.

>> - There are some explanation how to deal with multivariate statistics
>>   in "14.1 Using Explain" and "14.2 Statistics used by the Planner"
>>   section.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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] [COMMITTERS] pgsql: Bloom index contrib module

[Tom Lane]

I wrote:
> Would it be possible to dial down the amount of runtime consumed by
> the regression tests for this module?

Hmm ... "perf" says that a full 50% of the runtime of contrib/bloom's
regression test is consumed by GenericXLogFinish, and of that, the vast
majority is consumed by the extremely inefficient one-byte-at-a-time
matching in writeDelta() --- even dwarfing the "exchange two page images"
memcpy's.  So there seems to be good reason to expend some
micro-optimization effort in that area.  But aside from the writeDelta
problems, what in the world is the reason for the page exchange logic?
I could see copying a working image into place in the shared buffer
arena, but not saving the previous contents.

Even if these costs went to zero, though, the bloom regression tests
would still be consuming a disproportionate amount of time.

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] [COMMITTERS] pgsql: Bloom index contrib module

[Tom Lane]

Teodor Sigaev  writes:
> Bloom index contrib module

Would it be possible to dial down the amount of runtime consumed by
the regression tests for this module?

On my primary dev machine, "make installcheck" in contrib/bloom takes
4.5 seconds, which seems a bit excessive when make installcheck across
all the rest of contrib takes 22 seconds.

On prairiedog, which admittedly is one of the slower buildfarm animals
(though far from the slowest), the contrib-install-check-C step went
from 2:54 immediately before this patch went in to 4:28 immediately
after.

That seems out of line for a single contrib module, especially one of
unproven usefulness.

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] [patch] Proposal for \crosstabview in psql

[Daniel Verite]

Alvaro Herrera wrote:

> I pushed it.

That's awesome, thanks! Also thanks to Pavel who reviewed and helped
continuously when iterating on this feature, and all others who
participed in this thread.

Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


-- 
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] VS 2015 support in src/tools/msvc

[Christian Ullrich]

> Michael Paquier wrote:
> 
> On Sat, Apr 9, 2016 at 7:41 AM, Michael Paquier
>  wrote:
>> On Sat, Apr 9, 2016 at 1:46 AM, Christian Ullrich  
>> wrote:
>>> * Andrew Dunstan wrote:
> On 04/08/2016 11:02 AM, Christian Ullrich wrote:
>  src/port/chklocale.c(233): warning C4133: 'function': incompatible
>  types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]
>>> 
 Do you have a fix for the LPCWSTR parameter issue?
>>> 
>>> As long as the locale short name cannot contain characters outside of ASCII,
>>> and I don't see how it could, just the typical measure-allocate-convert
>>> dance, add error handling to taste:
>>> 
>>> int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
>>> WCHAR *wctype = malloc(res * sizeof(WCHAR));
>>> memset(wctype, 0, res * sizeof(WCHAR));
>>> res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);
> 
> I don't think that's good to use malloc in those code paths, and I
> think that we cannot use palloc as well for a buffer passed directly
> into this function, so it looks that we had better use a fix-sized
> buffer and allocate the output into that. What would be a a correct
> estimation of the maximum size we should allow? 80 (similar to
> pg_locale.c)?

I think it should not take more than 16 characters, but if you want to make 
sure the code can deal with long names as well, MSDN gives an upper limit of 85 
for those somewhere. 

-- 
Christian

-- 
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] VS 2015 support in src/tools/msvc

[Michael Paquier]

On Sat, Apr 9, 2016 at 7:41 AM, Michael Paquier
 wrote:
> On Sat, Apr 9, 2016 at 1:46 AM, Christian Ullrich  
> wrote:
>> * Andrew Dunstan wrote:
>>> On 04/08/2016 11:02 AM, Christian Ullrich wrote:
   src/port/chklocale.c(233): warning C4133: 'function': incompatible
   types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]
>>
>>> Do you have a fix for the LPCWSTR parameter issue?
>>
>> As long as the locale short name cannot contain characters outside of ASCII,
>> and I don't see how it could, just the typical measure-allocate-convert
>> dance, add error handling to taste:
>>
>> int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
>> WCHAR *wctype = malloc(res * sizeof(WCHAR));
>> memset(wctype, 0, res * sizeof(WCHAR));
>> res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);

I don't think that's good to use malloc in those code paths, and I
think that we cannot use palloc as well for a buffer passed directly
into this function, so it looks that we had better use a fix-sized
buffer and allocate the output into that. What would be a a correct
estimation of the maximum size we should allow? 80 (similar to
pg_locale.c)?
-- 
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] closing CommitFest 2016-03, feature freeze now in effect

[Simon Riggs]

On 9 April 2016 at 11:57, Stas Kelvich  wrote:

>
> > On 09 Apr 2016, at 03:05, Robert Haas  wrote:
> >
> > CommitFest 2016-03 is now closed.  I have moved "Twophase transactions
> > on slave", "Partial sort", and "amcheck (B-Tree integrity checking
> > tool)" to the next CommitFest in accordance with the policy previous
> > set by the release management team.   I have left "Replace buffer
> > manager spinlock with atomic operations" active in the current
> > CommitFest because it was granted an extension.  The RMT has received
> > Tom's request for an extension on the "Unique Joins" patch but has not
> > yet reached a decision.
> >
>
> Aren’t  "Twophase transactions on slave” falling into category of patches
> that fixes
> previously introduces behaviour? |'m not trying to argue with RMT
> decision, but just
> want to ensure that it was thoughtful decision, taking into account that
> absence of that
> patch in release can cause problems with replication in some cases as it
> was warned
> by Jesper[1] and Andres[2].
>
> [1] http://www.postgresql.org/message-id/5707a8cc.6080...@redhat.com
>
> [2]
> http://www.postgresql.org/message-id/80856693-5065-4392-8606-cf572a2ff...@anarazel.de


It's a longstanding problem and it would be good if we had an improvement.

I can't commit a patch that has a reported bug against it, nor can we fix
the problem if we can't reproduce it.

If we do get a committable patch, that is then the time to make a case to
RMT, but not before.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] multivariate statistics v14

[Tomas Vondra]

Hi,

On 04/09/2016 01:21 AM, Tatsuo Ishii wrote:

From: Tomas Vondra 

...

My feedback regarding docs were:

- There's no docs for pg_mv_statistic (should be added to "49. System
  Catalogs")

- The word "multivariate statistics" or something like that should
  appear in the index.

- There are some explanation how to deal with multivariate statistics
  in "14.1 Using Explain" and "14.2 Statistics used by the Planner"
  section.


The second and the third point maybe are something like "polishing
user-level" docs, but I don't think the first one is for "user-level".
Also I think without the first one the patch will be never
committable. If someone add a new system catalog, the doc should be
added to "System Catalogs" section, that's our standard, at least in
my understanding.


I do apologize if it seemed that I don't value your review, and I do 
agree that those changes need to be done, although I still see them 
rather as a user-level docs (as opposed to READMEs/comments, which I 
think are used by developers much more often).


But I still think it wouldn't move the patch any closer to committable 
state, because what it really needs is review whether the catalog 
definition makes sense, whether it should be more like pg_statistic, and 
so on. Only then it makes sense to describe the catalog structure in the 
SGML docs, I think. That's why I added some basic SGML docs for 
CREATE/DROP/ALTER STATISTICS, which I expect to be rather stable, and 
not the catalog and other low-level stuff (which is commented heavily in 
the code anyway).


Had the patch been a Titanic, fixing the SGML docs a few days before the 
code freeze would be akin to washing the deck instead of looking for 
icebergs on April 15, 1912.


regards

--
Tomas Vondra  http://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] Performance improvement for joins where outer side is unique

[Robert Haas]

On Thu, Apr 7, 2016 at 7:59 PM, Tom Lane  wrote:
> Anyway, I think it would be reasonable to give this patch a few more
> days in view of David's being away through the weekend.  But the RMT
> has final say on that.

The RMT has considered this request (sorry for the delay) and thought
it had merit, but ultimately voted not to give this patch an
extension.

Robert Haas
PostgreSQL 9.6 Release Management Team


-- 
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] closing CommitFest 2016-03, feature freeze now in effect

[Robert Haas]

On Sat, Apr 9, 2016 at 6:57 AM, Stas Kelvich  wrote:
> Aren’t  "Twophase transactions on slave” falling into category of patches 
> that fixes
> previously introduces behaviour? |'m not trying to argue with RMT decision, 
> but just
> want to ensure that it was thoughtful decision, taking into account that 
> absence of that
> patch in release can cause problems with replication in some cases as it was 
> warned
> by Jesper[1] and Andres[2].

The RMT hasn't made a specific decision on this patch.  I merely moved
it in accordance with the general RMT decision about feature freeze:

http://www.postgresql.org/message-id/ca+tgmoy56w5fozeeo+i48qehl+bsvtwy-q1m0xjuhucwggw...@mail.gmail.com

My personal view is as follows:

If the patch sped up things on the master but not on the slave, that
doesn't justify a post-freeze change to speed up the slave.  That can
be done for 9.7.  On the other hand, if the patch broke things that
are supposed to work, then that must be fixed.

-- 
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] closing CommitFest 2016-03, feature freeze now in effect

[Stas Kelvich]

> On 09 Apr 2016, at 03:05, Robert Haas  wrote:
> 
> CommitFest 2016-03 is now closed.  I have moved "Twophase transactions
> on slave", "Partial sort", and "amcheck (B-Tree integrity checking
> tool)" to the next CommitFest in accordance with the policy previous
> set by the release management team.   I have left "Replace buffer
> manager spinlock with atomic operations" active in the current
> CommitFest because it was granted an extension.  The RMT has received
> Tom's request for an extension on the "Unique Joins" patch but has not
> yet reached a decision.
> 

Aren’t  "Twophase transactions on slave” falling into category of patches that 
fixes
previously introduces behaviour? |'m not trying to argue with RMT decision, but 
just
want to ensure that it was thoughtful decision, taking into account that 
absence of that
patch in release can cause problems with replication in some cases as it was 
warned
by Jesper[1] and Andres[2].

[1] http://www.postgresql.org/message-id/5707a8cc.6080...@redhat.com

[2] 
http://www.postgresql.org/message-id/80856693-5065-4392-8606-cf572a2ff...@anarazel.de

Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres 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] Combining Aggregates

[David Rowley]

On 9 April 2016 at 05:49, Robert Haas  wrote:
> On Fri, Apr 8, 2016 at 11:57 AM, Robert Haas  wrote:
>> On Wed, Apr 6, 2016 at 5:28 PM, David Rowley
>>  wrote:
 Is that everything now? I don't see anything about combining aggs in the 
 git
 log and this is still showing as UnCommitted in the CF app.
>>>
>>> There's just a documentation patch and two combine functions for
>>> floating point aggs left now (Haribabu's patch)
>>
>> I'll go have a look at that now.  Hopefully it will be in good shape
>> and committable; if not, it'll have to wait for 9.7.
>
> And... it's pretty hard to argue with any of this.  Committed.
>
> I am *so* glad to be done with this patch series.  Man, that was a lot of 
> work.

Great!

Thanks for committing all these Robert. I really appreciate it, and I
know I'm not the only one.

-- 
 David Rowley   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] multivariate statistics v14

[Simon Riggs]

On 8 April 2016 at 20:13, Tom Lane  wrote:


> I will make it a high priority for 9.7, though.
>

That is my plan also. I've already started reviewing the non-planner parts
anyway, specifically patch 0002.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] Move PinBuffer and UnpinBuffer to atomics

[Alexander Korotkov]

On Fri, Apr 8, 2016 at 10:19 PM, Alexander Korotkov <
a.korot...@postgrespro.ru> wrote:

> On Fri, Apr 8, 2016 at 7:39 PM, Andres Freund  wrote:
>
>> As you can see in
>>
>
>> http://archives.postgresql.org/message-id/CA%2BTgmoaeRbN%3DZ4oWENLvgGLeHEvGZ_S_Z3KGrdScyKiSvNt3oA%40mail.gmail.com
>> I'm planning to apply this sometime this weekend, after running some
>> tests and going over the patch again.
>>
>> Any chance you could have a look over this?
>>
>
> I took a look at this.  Changes you made look good for me.
> I also run test on 4x18 Intel server.
>

On the top of current master results are following:

clientsTPS
1 12562
2 25604
4 52661
8103209
10   128599
20   256872
30   365718
40   432749
50   513528
60   684943
70   696050
80   923350
90  1119776
100 1208027
110 1229429
120 1163356
130 1107924
140 1084344
150 1014064
160  961730
170  980743
180  968419

The results are quite discouraging because previously we had about 1.5M TPS
in the peak while we have only about 1.2M now.  I found that it's not
related to the changes you made in the patch, but it's related to 5364b357
"Increase maximum number of clog buffers".  I'm making same benchmark with
5364b357 reverted.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company