Re: [HACKERS] [COMMITTERS] pgsql: Code cleanup in the wake of recent LWLock refactoring.

[Simon Riggs]

On 10 February 2016 at 16:36, Tom Lane  wrote:

> Robert Haas  writes:
> > On Wed, Feb 10, 2016 at 11:08 AM, Heikki Linnakangas 
> wrote:
> >> (Sorry if this was discussed already, I haven't been paying attention)
> >>
> >> LWLockAssign() is used by extensions. Are we OK with just breaking them,
> >> requiring them to change LWLockAssign() with the new mechanism, with
> #ifdefs
> >> to support multiple server versions? Seems like it shouldn't be too
> hard to
> >> keep LWLockAssign() around for the benefit of extensions, so it seems a
> bit
> >> inconsiderate to remove it.
>
> > If there's a strong feeling that we should keep the old APIs around,
> > we can do that, but I think that (1) if we don't remove them now, we
> > probably never will and (2) they are vile APIs.  Allocating the number
> > of add-in lwlocks that are requested or a minimum of 3 is just awful.
> > If somebody allocates a different number than they request it
> > sometimes works, except when combined with some other extension, when
> > it maybe doesn't work.  This way, you ask for an LWLock under a given
> > name and then get it under that name, so if an extension does it
> > wrong, it is that extension that breaks rather than some other one.  I
> > think that's enough benefit to justify requiring a small code change
> > on the part of extension authors that use LWLocks, but that's
> > obviously biased by my experience maintaining EDB's extensions, and
> > other people may well feel differently.
>
> FWIW, I wasn't paying attention either, but I'm convinced by Robert's
> argument.  Avoiding coupling between extensions is worth an API break.
>

I don't buy that, sorry.

New features, new APIs, very much agreed. Better API is a reason for new
api, not a reason to remove old.

Old APIs - why can't we keep it? The change is minor, so it we can easily
map old to new. Why choose to break all extensions that do this? We could
easily keep this by making the old API assign locks out of a chunk called
"Old Extension API". Deprecate the old API and remove in a later release.
Like pretty much every other API we support.

We must respect that Extension authors need to support a number of our
releases, meaning their job is already complex enough. We want to support
people that write extensions, not throw obstacles in their path,

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

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

Re: [HACKERS] Way to check whether a particular block is on the shared_buffer?

[Kouhei Kaigai]

> -Original Message-
> From: pgsql-hackers-ow...@postgresql.org
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas
> Sent: Saturday, February 13, 2016 1:46 PM
> To: Kaigai Kouhei(海外 浩平)
> Cc: Jim Nasby; pgsql-hackers@postgresql.org; Amit Langote
> Subject: Re: [HACKERS] Way to check whether a particular block is on the
> shared_buffer?
> 
> On Thu, Feb 11, 2016 at 9:05 PM, Kouhei Kaigai  wrote:
> > Hmm. In my experience, it is often not a productive discussion whether
> > a feature is niche or commodity. So, let me change the viewpoint.
> >
> > We may utilize OS-level locking mechanism here.
> >
> > Even though it depends on filesystem implementation under the VFS,
> > we may use inode->i_mutex lock that shall be acquired during the buffer
> > copy from user to kernel, at least, on a few major filesystems; ext4,
> > xfs and btrfs in my research. As well, the modified NVMe SSD driver can
> > acquire the inode->i_mutex lock during P2P DMA transfer.
> >
> > Once we can consider the OS buffer is updated atomically by the lock,
> > we don't need to worry about corrupted pages, but still needs to pay
> > attention to the scenario when updated buffer page is moved to GPU.
> >
> > In this case, PD_ALL_VISIBLE may give us a hint. GPU side has no MVCC
> > infrastructure, so I intend to move all-visible pages only.
> > If someone updates the buffer concurrently, then write out the page
> > including invisible tuples, PD_ALL_VISIBLE flag shall be cleared because
> > updated tuples should not be visible to the transaction which issued
> > P2P DMA.
> >
> > Once GPU met a page with !PD_ALL_VISIBLE, it can return an error status
> > that indicates CPU to retry this page again. In this case, this page is
> > likely loaded to the shared buffer already, so retry penalty is not so
> > much.
> >
> > I'll try to investigate the implementation in this way.
> > Please correct me, if I misunderstand something (especially, treatment
> > of PD_ALL_VISIBLE).
> 
> I suppose there's no theoretical reason why the buffer couldn't go
> from all-visible to not-all-visible and back to all-visible again all
> during the time you are copying it.
>
The backend process that is copying the data to GPU has a transaction
in-progress (= not committed). Is it possible to get the updated buffer
page back to the all-visible state again?
I expect that in-progress transactions works as a blocker for backing
to all-visible. Right?

> Honestly, I think trying to access buffers without going through
> shared_buffers is likely to be very hard to make correct and probably
> a loser.
>
No challenge, no outcome. ;-)

> Copying the data into shared_buffers and then to the GPU is,
> doubtless, at least somewhat slower.  But I kind of doubt that it's
> enough slower to make up for all of the problems you're going to have
> with the approach you've chosen.
>
Honestly, I'm still uncertain whether it works well as I expects.
However, scan workload on the table larger than main memory is
headache for PG-Strom, so I'd like to try ideas we can implement.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 


-- 
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] WIP: SCRAM authentication

[Michael Paquier]

On Sat, Feb 13, 2016 at 3:05 AM, David Steele  wrote:
> On 11/16/15 8:53 AM, Michael Paquier wrote:
>> On Sat, Sep 5, 2015 at 9:31 AM, Bruce Momjian wrote:
>>> On Fri, Sep  4, 2015 at 04:51:33PM -0400, Stephen Frost wrote:
> Coming in late, but can you explain how multiple passwords allow for
> easier automated credential rotation?  If you have five applications
> with stored passwords, I imagine you can't change them all at once, so
> with multiples you could change it on one, then go to the others and
> change it there, and finally, remove the old password.  Is that the
> process?  I am not realizing that without multiple plasswords, this is a
> hard problem.
 That's exactly the process if multiple passwords can be used.  If
 there's only one account and one password supported then you have to
 change all the systems all at once and that certainly can be a hard
 problem.

 One way to deal with this is to have a bunch of different accounts, but
 that's certainly not simple either and can get quite painful.
>>> OK, for me, if we can explain the benefit for users, it seems worth
>>> doing just to allow that.
>> Reviving an old thread for a patch still registered in this commit
>> fest to make the arguing move on.
>
> I was wondering if this patch was going to be submitted for the 2016-03 CF?

For 9.6 I am afraid this is too late, per the rule that there cannot
be huge patches for the last CF of a development cycle. But I have
plans for this set of features afterwards with the first CF of 9.7 and
I was planning to talk about it at PgCon's unconference if I can get
there to gather some feedback. There is still cruel need for it on my
side..

> If so I am interesting in testing/reviewing or doing any other work that
> would be helpful.

Thanks.

> In addition, I would prefer to maintain the current structure of the
> pg_authid table and use the rolpassword and rolvaliduntil columns to
> store only the md5 verifier which will also be stored in
> pg_auth_verifiers.  This would provide a smoother migration path with
> the idea that rolpassword and rolvaliduntil will be removed from
> pg_authid in a future version of Postgres.

Actually, I am of the opinion that both rolpassword and rolvaliduntil
should be directly part of pg_auth_verifiers. Being able to handle
multiple verifiers for the same protocol is a feature that is being
asked for with a given password handling policy (was pinged again
about that in Moscow last week). Rolling in new verifiers needs now
extra roles to be created.

>> There are clear concerns about protocol aging and how to facilitate
>> the life of users to switch to a new system. Hence, I think that the
>> patch could include the following:
>> - A compatibility GUC allowed_password_verifiers defaulting to a list
>> of verifier protocols that we think are safe. This would be added in
>> the patch adding infrastructure for multiple verifiers, with default
>> to md5. In the patch adding SCRAM, the value of this parameter is
>> changed to md5,scram. If a user create a password verifier with a
>> protocol not listed in this parameter we return to him a WARNING.
>> ERROR may be too much but opinions are welcome.
>
> It seems like an error would be better here.

Noted.

>> - A superuser cleanup function for pg_auth_verifiers aimed at being
>> used by pg_upgrade to do needed cleanup of this catalog based on the
>> previous GUC to remove outdated verifiers. Optionally, we could have
>> an argument for a list of protocols to clean up.
>> Using both things we could leverage the upgrade experience and
>> transition between systems. Say even if at some point we decide to
>> decommission SCRAM we could reuse the same infrastructure to move on
>> to a new major version.
>
> Yes - and although the eventual migration process may not need to be
> worked out it its entirety we should have a very good idea of what it's
> going to look like as that will inform some of the decisions that need
> to be made now.

Thinking about that again a combination of a GUC and an interface
dedicated to pg_upgrade sounds the saner way of going here.

> Please let me know if there's anything I can do to expedite this patch.

I am planning to work on a new patch following the ideas I have sent
upthread, after the last CF of 9.6 is wrapped up. This thread is high
on my priority list.
-- 
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] Small PATCH: check of 2 Perl modules

[Michael Paquier]

On Sat, Feb 13, 2016 at 1:47 PM, Robert Haas  wrote:
> On Fri, Feb 12, 2016 at 8:20 AM, Eugene Kazakov
>  wrote:
>> TAP-tests need two Perl modules: Test::More and IPC::Run.
>>
>> The patch adds checking of modules in configure.in and configure.
>
> Why would we want that?

I was doubtful at the beginning, but it doesn't hurt to have those
sanity checks in configure actually. The idea is that when
--enable-tap-tests is used now we simply error with "Can't locate
IPC/Run.pm in @INC" when kicking the tests, this check would allow one
to know if his environment is adapted to run the tests or not before
compiling anything.

And with this patch, we would fail now with that:
configure: error: Need Perl IPC::Run module

By the way, the patch given upthread by Eugene is incorrect. To begin
with, AX_PROG_PERL_MODULES has not been compiled by autoconf and I
can't believe that it is available on all platforms, for example on
OSX 10.8 I could not see it. And it is actually here:
https://www.gnu.org/software/autoconf-archive/ax_prog_perl_modules.html

I would recommend grabbing a copy of this file, and change the error
message as follows:
Perl module IPC::Run is required to run TAP tests

See the patch attached as reference, we could simplify the macro of
this m4 file and remove the check for perl, which is here:
+# Make sure we have perl
+if test -z "$PERL"; then
+AC_CHECK_PROG(PERL,perl,perl)
+fi
Though I kept the original script as-is in the patch attached.
Regards,
-- 
Michael
diff --git a/aclocal.m4 b/aclocal.m4
index 6f930b6..db11141 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -1,5 +1,6 @@
 dnl aclocal.m4
 m4_include([config/ac_func_accept_argtypes.m4])
+m4_include([config/ax_prog_perl_modules.m4])
 m4_include([config/ax_pthread.m4])
 m4_include([config/c-compiler.m4])
 m4_include([config/c-library.m4])
diff --git a/config/ax_prog_perl_modules.m4 b/config/ax_prog_perl_modules.m4
new file mode 100644
index 000..11a326c
--- /dev/null
+++ b/config/ax_prog_perl_modules.m4
@@ -0,0 +1,77 @@
+# ===
+#   http://www.gnu.org/software/autoconf-archive/ax_prog_perl_modules.html
+# ===
+#
+# SYNOPSIS
+#
+#   AX_PROG_PERL_MODULES([MODULES], [ACTION-IF-TRUE], [ACTION-IF-FALSE])
+#
+# DESCRIPTION
+#
+#   Checks to see if the given perl modules are available. If true the shell
+#   commands in ACTION-IF-TRUE are executed. If not the shell commands in
+#   ACTION-IF-FALSE are run. Note if $PERL is not set (for example by
+#   calling AC_CHECK_PROG, or AC_PATH_PROG), AC_CHECK_PROG(PERL, perl, perl)
+#   will be run.
+#
+#   MODULES is a space separated list of module names. To check for a
+#   minimum version of a module, append the version number to the module
+#   name, separated by an equals sign.
+#
+#   Example:
+#
+# AX_PROG_PERL_MODULES( Text::Wrap Net::LDAP=1.0.3, ,
+#   AC_MSG_WARN(Need some Perl modules)
+#
+# LICENSE
+#
+#   Copyright (c) 2009 Dean Povey 
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 7
+
+AU_ALIAS([AC_PROG_PERL_MODULES], [AX_PROG_PERL_MODULES])
+AC_DEFUN([AX_PROG_PERL_MODULES],[dnl
+
+m4_define([ax_perl_modules])
+m4_foreach([ax_perl_module], m4_split(m4_normalize([$1])),
+	  [
+	   m4_append([ax_perl_modules],
+		 [']m4_bpatsubst(ax_perl_module,=,[ ])[' ])
+  ])
+
+# Make sure we have perl
+if test -z "$PERL"; then
+AC_CHECK_PROG(PERL,perl,perl)
+fi
+
+if test "x$PERL" != x; then
+  ax_perl_modules_failed=0
+  for ax_perl_module in ax_perl_modules; do
+AC_MSG_CHECKING(for perl module $ax_perl_module)
+
+# Would be nice to log result here, but can't rely on autoconf internals
+$PERL -e "use $ax_perl_module; exit" > /dev/null 2>&1
+if test $? -ne 0; then
+  AC_MSG_RESULT(no);
+  ax_perl_modules_failed=1
+   else
+  AC_MSG_RESULT(ok);
+fi
+  done
+
+  # Run optional shell commands
+  if test "$ax_perl_modules_failed" = 0; then
+:
+$2
+  else
+:
+$3
+  fi
+else
+  AC_MSG_WARN(could not find perl)
+fi])dnl
diff --git a/configure b/configure
index b3f3abe..3613fd6 100755
--- a/configure
+++ b/configure
@@ -15554,6 +15554,161 @@ done
   if test -z "$PERL"; then
 as_fn_error $? "Perl not found" "$LINENO" 5
   fi
+  # Check for necessary modules
+
+
+
+
+
+
+# Make sure we have perl
+if test -z "$PERL"; then
+# Extract the first word of "perl", so it can be a program name with args.
+set dummy perl; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_PERL+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+ 

[HACKERS] Defaults for replication/backup

[Magnus Hagander]

I know we've had many discussions about the defaults, so hey, let's bring
out the paint-cans and do the bikeshed all over again.

I would suggest a couple of changes to the default values in parameters
related to backup and replication.

The reason for this is to make it "easier to do the right thing by
default", meaning the defaults should be more suitable for new users to use
our tools in the default config.

Yes, these changes will increase some of the default overhead. My argument
against that is that anybody who actually cares about that overhead is
going to be tuning their database *anyway*, so they can just change things
back to the old defaults.

So, I suggest the following changes to the defaults:

wal_level=hot_standby
max_wal_senders=10
max_replication_slots=10

And in pg_hba.conf, we enable the replication connections by default for
the superuser on local/localhost.

This will make it possible to actually take a proper backup (not just a
dump) off a default config cluster without having to do a restart first.
And it allows you to set up archiving with pg_receivexlog for full PITR.

I think the overhead of the two max parameters is really small, but they're
both dependent on the default wal_level. Yes, we've discussed it before. I
think it's time to make the change now.

I'm sure there are many more settings we could discuss the defaults for,
but this particular biikeshed is mine so I get to decide the initial
borders, even if I don't get to decide the colors :P

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] extend pgbench expressions with functions

[Michael Paquier]

On Sat, Feb 13, 2016 at 4:37 PM, Fabien COELHO  wrote:
>
>> For example, I just realized that this patch allows values to be
>> either a double or an integer and extends the operators to handle
>> double values.  But variables can still only be integers.
>
> Indeed.

That's exactly the first impression I got about this patch when sending [1].
[1]: 
http://www.postgresql.org/message-id/cab7npqrhvfvnrahamaroniedec90pf3-wn+u5ccb4red-ht...@mail.gmail.com
By focusing only on integers the patch would largely gain in
simplicity. Now being able to have double values is actually useful
for nested function calls, nothing else.

>> I don't think variables should be explicitly typed but they should be able
>> to store a value of any type that expression evaluation can generate.
>
> Doubles are not really needed that much, it is just to provide something to
> random_* functions parameter, otherwise it is useless as far as pgbench is
> really concerned.

+1.

>> Also, as I said back in November, there's really two completely
>> separate enhancements in here.  One of them is to support a new data
>> type (doubles) and the other is to support functions.
>
> Yep. The first part is precisely the patch I initially submitted 5 CF ago.
> Then I'm asked to put more things in it to show that it can indeed handle
> another type. Then I'm told "you should not have done that". What can I say?

I think that you did your job here by answering the comments of all
the reviewers that had a look at this patch. That's not an easy task.

>> [...] I find implementing operators as functions in disguise not to be one
>> of PostgreSQL's most awesome design decisions, and here we are copying that
>> into pgbench for, well, no benefit that I can see, really.
>
> Well, I did that initially, then I was asked to implements operators as
> functions. It probably saves some lines, so it is not too bad, even if the
> benefit is limited.

Put the blame on me for this one then. I suggested this idea because
Postgres is doing the same, and because it simplifies slightly the
union structure in charge of holding the parsed structures, making the
code a bit more readable IMO.

>> [...] If neither of you are willing to split this patch, I'm not willing
>> to commit it.
>
> If I'm reading you correctly, you would consider committing it:
>
>  - if the function & double stuff are separated ?
>  - for the double part, if variables can be double ?

I just double-checked and could not see a clear use case mentioned in
this thread for double return types, so I would suggest focusing on
the integer portion with min(), max(), abs(), debug() and the existing
functions refactored. That's what your first versions did. If someone
is wishing to implement double types, this someone could do it, the
infrastructure that this patch puts in place has already proved that
it can be easily extended.
-- 
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] Defaults for replication/backup

[Michael Paquier]

On Sat, Feb 13, 2016 at 10:15 PM, Magnus Hagander wrote:
> So, I suggest the following changes to the defaults:
> wal_level=hot_standby
> max_wal_senders=10
> max_replication_slots=10

10 seems a bit high. I would think that max_wal_senders and
max_replication_slots set at 3 are sufficient enough, giving enough
room for simple deployments willing to use replication.

> And in pg_hba.conf, we enable the replication connections by default for the
> superuser on local/localhost.

+1 on that.
-- 
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] Defaults for replication/backup

[Andres Freund]

On 2016-02-13 22:37:33 +0900, Michael Paquier wrote:
> On Sat, Feb 13, 2016 at 10:15 PM, Magnus Hagander wrote:
> > So, I suggest the following changes to the defaults:
> > wal_level=hot_standby
> > max_wal_senders=10
> > max_replication_slots=10

+1. I'm inclined to set slots a bit higher than senders. They're pretty
cheap...

I think this is way past due.

> 10 seems a bit high. I would think that max_wal_senders and
> max_replication_slots set at 3 are sufficient enough, giving enough
> room for simple deployments willing to use replication.

But they're relatively cheap to enable, so why set them low?


Andres


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

[HACKERS] Re: [COMMITTERS] pgsql: Add some isolation tests for deadlock detection and resolution.

[Greg Stark]

The tests worked fine on faster build animals, right? And the clobber
animals are much much slower So it seems perfectly sensible that their
deadlock timeout would just have to be much much higher to have the same
behaviour. I see nothing wrong in just setting deadlock timeout to a minute
or more on these machines.

The invariant is just that the deadlock timeout needs enough head room over
the actual time the tester's queries take. If they normally take a 1/10th
of a second then why not just set the timeout to 10x however long they take
on the clobber animals?

-- 
Greg

Re: [HACKERS] GinPageIs* don't actually return a boolean

[Michael Paquier]

On Sat, Feb 13, 2016 at 1:48 AM, Tom Lane  wrote:
> Andres Freund  writes:
>> On February 12, 2016 5:29:44 PM GMT+01:00, Tom Lane  
>> wrote:
>>> We should standardize on the "((var & FLAG) != 0)"
>>> pattern, which works reliably in all cases.
>
>> That's what the second version of my patch, and I presume Michael's updated 
>> one as well, does. I think the only open question is how far to backpatch. 
>> While annoying work, I think we should go all the way.
>
> I don't object to that, if someone wants to do the work.  A good argument
> for it is that we'd otherwise be laying a nasty trap for future
> back-patched bug fixes, which might well rely on the cleaned-up behavior.

>From the MSVC-only perspective, that's down to 9.3, but it would
definitely make sense to backpatch 2 versions further down to
facilitate future bug fix integration, so +1 to get that down to 9.1.
Andres, I guess that you are on that? That's your patch after all.
-- 
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] Defaults for replication/backup

[Magnus Hagander]

On Sat, Feb 13, 2016 at 2:39 PM, Andres Freund  wrote:

> On 2016-02-13 22:37:33 +0900, Michael Paquier wrote:
> > On Sat, Feb 13, 2016 at 10:15 PM, Magnus Hagander wrote:
> > > So, I suggest the following changes to the defaults:
> > > wal_level=hot_standby
> > > max_wal_senders=10
> > > max_replication_slots=10
>
> +1. I'm inclined to set slots a bit higher than senders. They're pretty
> cheap...
>

So something like 10/20?



> I think this is way past due.
>
> > 10 seems a bit high. I would think that max_wal_senders and
> > max_replication_slots set at 3 are sufficient enough, giving enough
> > room for simple deployments willing to use replication.
>
> But they're relatively cheap to enable, so why set them low?
>
>
That's my reasoning as well. I think you will be hard pressed to even
measure the overhead between 3 and 10. The big difference will be in
leaving 0 and in particular setting wal_level.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] Defaults for replication/backup

[Joshua D. Drake]

On 02/13/2016 05:37 AM, Michael Paquier wrote:

On Sat, Feb 13, 2016 at 10:15 PM, Magnus Hagander wrote:

So, I suggest the following changes to the defaults:
wal_level=hot_standby
max_wal_senders=10
max_replication_slots=10


10 seems a bit high. I would think that max_wal_senders and
max_replication_slots set at 3 are sufficient enough, giving enough
room for simple deployments willing to use replication.


Q. Is there a demonstrable, noticeable downside to 10?
A. No

Great, let's make it easier for everyone.

+1




And in pg_hba.conf, we enable the replication connections by default for the
superuser on local/localhost.


+1 on that.


+1

JD



--
Command Prompt, Inc.  http://the.postgres.company/
 +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


--
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] extend pgbench expressions with functions

[Fabien COELHO]

Hello Michaël,


 - if the function & double stuff are separated ?
 - for the double part, if variables can be double ?


I just double-checked and could not see a clear use case mentioned in
this thread for double return types,


Alas there is one: non uniform random functions which use a double 
parameter.


Once random functions are there, the \setrandom horror code could be 
removed, which would be a real benefit, IMO:-)


So I see a good case to have some support for doubles.

so I would suggest focusing on the integer portion with min(), max(), 
abs(), debug() and the existing functions refactored. That's what your 
first versions did. If someone is wishing to implement double types, 
this someone could do it, the infrastructure that this patch puts in 
place has already proved that it can be easily extended.


Adding double is not too big a deal. I just stopped at variables because I 
could not see any realistic use for them. My idea was to postpone that 
till it is actually needed, "never" being the most probable course.


Now if this is a blocker for the committer, then I will probably make 
the effort whatever I think of the usefulness of the feature.


--
Fabien.
--
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: Code cleanup in the wake of recent LWLock refactoring.

[Tom Lane]

Simon Riggs  writes:
> On 10 February 2016 at 16:36, Tom Lane  wrote:
>> FWIW, I wasn't paying attention either, but I'm convinced by Robert's
>> argument.  Avoiding coupling between extensions is worth an API break.

> Old APIs - why can't we keep it?

Because with the old API, a bug in extension A may go unnoticed in A's
testing but break when it's combined with extension B.  That causes
headaches all around, not just to the extension authors but to their
users.  The new API ensures detection of didn't-request-enough-locks
bugs regardless of which other extensions are installed.  That is worth
the cost of a forced API update, in Robert's judgement and mine too.

(Having said that, I wonder if we could put back the old API as a shim
layer *without* the allocate-some-excess-locks proviso.  That would
get us to a situation where standalone testing of a broken extension
would disclose its bug, without breaking non-buggy extensions.)

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] Crash with old Windows on new CPU

[Tom Lane]

Christian Ullrich  writes:
> * Robert Haas wrote:
>> Thanks for the report and patch.  Regrettably I haven't the Windows
>> knowledge to have any idea whether it's right or wrong, but hopefully
>> someone who knows Windows will jump in here.

> In commitfest now.

FWIW, I'm a tad suspicious of the notion that it's our job to make this
case work.  How practical is it really to run a Windows release on
unsupported-by-Microsoft hardware --- aren't dozens of other programs
going to have the same issue?

I'm also suspicious of the "#if _MSC_VER == 1800" tests, that is,
the code compiles on *exactly one* MSVC version.  Maybe that's actually
what's needed, but it sure looks fishy.  And what connection does the
build toolchain version have to the runtime environment anyway?

Likewise, how can we know that !IsWindows7SP1OrGreater() is the exactly
right runtime test?

Lastly, I'd like to see some discussion of what side effects
"_set_FMA3_enable(0);" has ... I rather doubt that it's really
a magic-elixir-against-crashes-with-no-downsides.  That would
give us some context to estimate the risks of this code executing
when it's not really needed.  Without that, I'd be worried that
this cure is worse than the disease because it breaks cases that
weren't broken before.

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] Defaults for replication/backup

[Joshua D. Drake]

Hello,

I would like to add the idea of having archiving on by default. Not 
everyone uses streaming replication, some people use PITR. The one that 
I see is archive_command and I am not sure how to deal with that.


Sincerely,

JD

--
Command Prompt, Inc.  http://the.postgres.company/
 +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


--
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] Re: [COMMITTERS] pgsql: Add some isolation tests for deadlock detection and resolution.

[Tom Lane]

Greg Stark  writes:
> The tests worked fine on faster build animals, right? And the clobber
> animals are much much slower So it seems perfectly sensible that their
> deadlock timeout would just have to be much much higher to have the same
> behaviour. I see nothing wrong in just setting deadlock timeout to a minute
> or more on these machines.

We don't have a way to make the isolation tests change behavior depending
on how the backend is compiled.  So the only actually available fix is to
make that test take "a minute or more" for *everybody*.

Aside from that, it's just disturbing that these tests aren't
deterministic regardless of machine speed.  We don't seem to have a
way around that right now, but I wish we did.

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] Defaults for replication/backup

[Andres Freund]

Hi,

On 2016-02-13 07:13:55 -0800, Joshua D. Drake wrote:
> I would like to add the idea of having archiving on by default. Not everyone
> uses streaming replication, some people use PITR. The one that I see is
> archive_command and I am not sure how to deal with that.

Since that requires additional setup anyway, I don't really see a
comparable effect of such a move.

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] Defaults for replication/backup

[Magnus Hagander]

On Sat, Feb 13, 2016 at 4:16 PM, Andres Freund  wrote:

> Hi,
>
> On 2016-02-13 07:13:55 -0800, Joshua D. Drake wrote:
> > I would like to add the idea of having archiving on by default. Not
> everyone
> > uses streaming replication, some people use PITR. The one that I see is
> > archive_command and I am not sure how to deal with that.
>
> Since that requires additional setup anyway, I don't really see a
> comparable effect of such a move.
>

Also, you can reasonably do PITR without using
archive_command/archive_mode. Just use pg_receivexlog...

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] pg_basebackup vs WAL fetching

[Craig Ringer]

On 12 February 2016 at 22:37, Magnus Hagander  wrote:

> Right now, pg_basebackup can only use replication slots with WAL
> streaming. It's intended, I believe, to be used when you use pg_basebackup
> to set up a new replica, to close the gap before starting a replica (ref
> http://www.postgresql.org/message-id/flat/555dd2b2.7020...@gmx.net).
>
> A bit down that thread there is a discussion about pg_basebackup being
> able to use this for standalone backups, specifically -X mode, to make sure
> things are not rotated away.
>
> AFAICT this was never done, with the thread ending on a subject of "this
> is for a different thread". I couldn't find such a different thread.
>
> I think it would still be a very useful feature. In fact, I think it would
> be useful enough that it should be enabled by default (but of course
> possible to turn off), in the interest of making the default more difficult
> to break.
>
> On the backend side, I think that would just mean adding a parameter to 
> CREATE_REPLICATION_SLOT
> to create the slot ephemeral. And register an exit handler that will remove
> it when the backup session disconnects as well (ephemeral slots go away on
> crash, but AIUI not on regular shutdown).
>
> With this, it should be useful for both -X fetch and -X stream, as long as
> pg_basebackup doesn't connect from the master until it has seen all it's
> WAL.
>
> Did I miss a thread somewhere saying this is not a good idea?
> 
>

I think it's a very good idea ... with a significant caveat.

Using a persistent replication slot greatly increases the risk of problems
if the backup fails partway though. The abandoned slot will not be dropped
automatically and will continue to pin the minimum required LSN, risking
the filling of pg_xlog .

Internally replication slots have an ephemeral form, where the slot is
dropped automatically on crash recovery. This isn't exposed directly to
users. It's used for logical replication slots while they're waiting for a
valid snapshot. Maybe something along those lines could be useful, but
limited to the lifetime of the connection? The downside is that this would
stop pg_basebackup from ever learning to reconnect and resume receiving WAL
where it left off.

I'm not really sure how to handle that. I don't want pg_basebackup failures
to risk breaking the master. On the other hand it's clearly good to make
backups more reliable and slots provide a useful way to do that. Letting
pg_basebackup reconnect and resume reading WAL when using a slot would be
nice.

Thoughts?

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: [HACKERS] proposal: PL/Pythonu - function ereport

[Pavel Stehule]

Hi

I am sending new version. Current version does:

1. the plpy utility functions can use all ErrorData fields,
2. there are no new functions,
3. via GUC plpythonu.legacy_custom_exception we can return previous behave,
4. only exception Error is raised with natural structure - no composite
value spidata.
5. fields: message, detail and hint are implicitly translated to string -
it decrease a necessity of legacy mode

Curent version doesn't do:
1. touch plpy exception classes.

A doc should be enhanced, but the code should be +/- final.

Regards

Pavel
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
new file mode 100644
index 015bbad..2bd9bcb
*** a/doc/src/sgml/plpython.sgml
--- b/doc/src/sgml/plpython.sgml
*** $$ LANGUAGE plpythonu;
*** 1341,1360 
Utility Functions

 The plpy module also provides the functions
!plpy.debug(msg),
!plpy.log(msg),
!plpy.info(msg),
!plpy.notice(msg),
!plpy.warning(msg),
!plpy.error(msg), and
!plpy.fatal(msg).elogin PL/Python
 plpy.error and
 plpy.fatal actually raise a Python exception
 which, if uncaught, propagates out to the calling query, causing
 the current transaction or subtransaction to be aborted.
 raise plpy.Error(msg) and
 raise plpy.Fatal(msg) are
!equivalent to calling
 plpy.error and
 plpy.fatal, respectively.
 The other functions only generate messages of different
--- 1341,1360 
Utility Functions

 The plpy module also provides the functions
!plpy.debug(exception_params),
!plpy.log(exception_params),
!plpy.info(exception_params),
!plpy.notice(exception_params),
!plpy.warning(exception_params),
!plpy.error(exception_params), and
!plpy.fatal(exception_params).elogin PL/Python
 plpy.error and
 plpy.fatal actually raise a Python exception
 which, if uncaught, propagates out to the calling query, causing
 the current transaction or subtransaction to be aborted.
 raise plpy.Error(msg) and
 raise plpy.Fatal(msg) are
!partial equivalent to calling
 plpy.error and
 plpy.fatal, respectively.
 The other functions only generate messages of different
*** $$ LANGUAGE plpythonu;
*** 1367,1372 
--- 1367,1397 

  

+ 
+The exception_params are
+[ message [, detail [, hint [, sqlstate  [, schema  [, table  [, column  [, datatype  [, constraint ].
+These parameters kan be entered as keyword parameters.
+The message, detail, hint
+are automaticly casted to string, other should be string.
+ 
+ 
+ CREATE FUNCTION raise_custom_exception() RETURNS void AS $$
+ plpy.error("custom exception message", "some info about exception", "hint for users")
+ $$ LANGUAGE plpythonu;
+ 
+ postgres=# select raise_custom_exception();
+ ERROR:  XX000: plpy.Error: custom exception message
+ DETAIL:  some info about exception
+ HINT:  hint for users
+ CONTEXT:  Traceback (most recent call last):
+   PL/Python function "raise_custom_exception", line 2, in module
+ plpy.error("custom exception message", "some info about exception", "hint for users")
+ PL/Python function "raise_custom_exception"
+ LOCATION:  PLy_elog, plpy_elog.c:132
+ 
+   
+ 
+   
 Another set of utility functions are
 plpy.quote_literal(string),
 plpy.quote_nullable(string), and
diff --git a/src/pl/plpython/expected/plpython_spi.out b/src/pl/plpython/expected/plpython_spi.out
new file mode 100644
index e715ee5..3daf181
*** a/src/pl/plpython/expected/plpython_spi.out
--- b/src/pl/plpython/expected/plpython_spi.out
***
*** 1,3 
--- 1,4 
+ --set plpythonu.legacy_custom_exception = true;
  --
  -- nested calls
  --
diff --git a/src/pl/plpython/expected/plpython_test.out b/src/pl/plpython/expected/plpython_test.out
new file mode 100644
index 7b76faf..bfe5c6f
*** a/src/pl/plpython/expected/plpython_test.out
--- b/src/pl/plpython/expected/plpython_test.out
*** select module_contents();
*** 50,55 
--- 50,96 
  
  CREATE FUNCTION elog_test() RETURNS void
  AS $$
+ plpy.debug('debug','some detail')
+ plpy.log('log','some detail')
+ plpy.info('info','some detail')
+ plpy.info()
+ plpy.info('the question', 42);
+ plpy.info('This is message text.',
+ detail = 'This is detail text',
+ hint = 'This is hint text.',
+ sqlstate = 'XX000',
+ schema = 'any info about schema',
+ table = 'any info about table',
+ column = 'any info about column',
+ datatype = 'any info about datatype',
+ constraint = 'any info about constraint')
+ plpy.notice('notice','some detail')
+ plpy.warning('warning','some detail')
+ plpy.error('stop on error', 'some detail','some hint')
+ $$ LANGUAGE plpythonu;
+ SELECT elog_test();
+ INFO:  info
+ DETAIL:  some detail
+ INFO:  missing error text
+ INFO:  the question
+ 

Re: [HACKERS] pg_basebackup vs WAL fetching

[Andres Freund]

On 2016-02-13 23:25:06 +0800, Craig Ringer wrote:
> Internally replication slots have an ephemeral form, where the slot is
> dropped automatically on crash recovery.

And upon release (including a http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] extend pgbench expressions with functions

[Fabien COELHO]

But variables can still only be integers.


Version 28 attached has double variables, although typing is based on 
guessing because it is just a string.


Any comment?

[...] two separate enhancements in here.  One of them is to support a 
new data type (doubles) and the other is to support functions.


The two features are highly intermix, so it can only be dependent patches, 
first to add a function infrastructure and probably some support for 
doubles altough it would not be used, then to add doubles & their 
functions.


A real pain is the documentation, because it means writing a documentation 
with only integer functions, then overwriting it with doubles. This is 
dumb work, really, for the sake of "a cleaner git history", the beauty of 
it no one will ever contemplate...


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index ade1b53..5d38c6f 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -793,20 +793,24 @@ pgbench  options  dbname
 
 
  
-  Sets variable varname to an integer value calculated
+  Sets variable varname to a value calculated
   from expression.
   The expression may contain integer constants such as 5432,
+  double constants such as 3.14159,
   references to variables :variablename,
   and expressions composed of unary (-) or binary operators
-  (+, -, *, /, %)
-  with their usual associativity, and parentheses.
+  (+, -, *, /,
+  %) with their usual associativity, function calls and
+  parentheses.
+   shows the available
+  functions.
  
 
  
   Examples:
 
 \set ntellers 10 * :scale
-\set aid (1021 * :aid) % (10 * :scale) + 1
+\set aid (1021 * random(1, 10 * :scale)) % (10 * :scale) + 1
 
 

@@ -826,66 +830,35 @@ pgbench  options  dbname
  
 
  
-  By default, or when uniform is specified, all values in the
-  range are drawn with equal probability.  Specifying gaussian
-  or  exponential options modifies this behavior; each
-  requires a mandatory parameter which determines the precise shape of the
-  distribution.
- 
+  
+   
+
+ \setrandom n 1 10 or \setrandom n 1 10 uniform
+ is equivalent to \set n random(1, 10) and uses a uniform
+ distribution.
+
+   
 
- 
-  For a Gaussian distribution, the interval is mapped onto a standard
-  normal distribution (the classical bell-shaped Gaussian curve) truncated
-  at -parameter on the left and +parameter
-  on the right.
-  Values in the middle of the interval are more likely to be drawn.
-  To be precise, if PHI(x) is the cumulative distribution
-  function of the standard normal distribution, with mean mu
-  defined as (max + min) / 2.0, with
-
- f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) /
-(2.0 * PHI(parameter) - 1.0)
-
-  then value i between min and
-  max inclusive is drawn with probability:
-  f(i + 0.5) - f(i - 0.5).
-  Intuitively, the larger parameter, the more
-  frequently values close to the middle of the interval are drawn, and the
-  less frequently values close to the min and
-  max bounds. About 67% of values are drawn from the
-  middle 1.0 / parameter, that is a relative
-  0.5 / parameter around the mean, and 95% in the middle
-  2.0 / parameter, that is a relative
-  1.0 / parameter around the mean; for instance, if
-  parameter is 4.0, 67% of values are drawn from the
-  middle quarter (1.0 / 4.0) of the interval (i.e. from
-  3.0 / 8.0 to 5.0 / 8.0) and 95% from
-  the middle half (2.0 / 4.0) of the interval (second and
-  third quartiles). The minimum parameter is 2.0 for
-  performance of the Box-Muller transform.
- 
+  
+   
+\setrandom n 1 10 exponential 3.0 is equivalent to
+\set n random_exponential(1, 10, 3.0) and uses an
+exponential distribution.
+   
+  
 
- 
-  For an exponential distribution, parameter
-  controls the distribution by truncating a quickly-decreasing
-  exponential distribution at parameter, and then
-  projecting onto integers between the bounds.
-  To be precise, with
-
-f(x) = exp(-parameter * (x - min) / (max - min + 1)) / (1.0 - exp(-parameter))
-
-  Then value i between min and
-  max inclusive is drawn with probability:
-  f(x) - f(x + 1).
-  Intuitively, the larger parameter, the more
-  frequently values close to min are accessed, and the
-  less frequently values close to max are accessed.
-  The closer to 0 parameter, the flatter (more uniform)
-  the access distribution.
-  A crude approximation of the distribution is that the most frequent 1%
-  values in the range, close to min, are drawn
-  parameter% of the time.
-  parameter value must be strictly positive.
+  
+   
+  

Re: [HACKERS] Crash with old Windows on new CPU

[Christian Ullrich]

On February 13, 2016 4:10:34 PM Tom Lane  wrote:

> Christian Ullrich  writes:
>> * Robert Haas wrote:
>>> Thanks for the report and patch.  Regrettably I haven't the Windows
>>> knowledge to have any idea whether it's right or wrong, but hopefully
>>> someone who knows Windows will jump in here.
>
>> In commitfest now.
>
> FWIW, I'm a tad suspicious of the notion that it's our job to make this
> case work.  How practical is it really to run a Windows release on
> unsupported-by-Microsoft hardware --- aren't dozens of other programs
> going to have the same issue?

Why would the hardware be unsupported? The problem occurs on new CPUs, not old 
ones, and even the OS (2008) is still in extended support until next year, IIRC.

> I'm also suspicious of the "#if _MSC_VER == 1800" tests, that is,
> the code compiles on *exactly one* MSVC version.  

The bug exists in only that compiler version's CRT, also, that is not the 
complete version number. There may be different builds somewhere, but they all 
start with 18.0. 

After all, MS is in the business of selling new compilers, not maintaining the 
old ones. 

> Maybe that's actually
> what's needed, but it sure looks fishy.  And what connection does the
> build toolchain version have to the runtime environment anyway?

The CRT version is tied to the compiler version. It has mainly to do with 
matching allocators.

> Likewise, how can we know that !IsWindows7SP1OrGreater() is the exactly
> right runtime test?

Because all sources, including Microsoft, say that AVX2 support was added in 
7SP1.

> Lastly, I'd like to see some discussion of what side effects
> "_set_FMA3_enable(0);" has ... I rather doubt that it's really
> a magic-elixir-against-crashes-with-no-downsides.  

It tells the math library (in the CRT, no separate libm on Windows) not to use 
the AVX2-based implementations of log() and possibly other functions. AIUI, FMA 
means "fused multiply-add" and is apparently something that increases 
performance and accuracy in transcendental functions.

I can check the CRT source later today and figure out exactly what it does. 

Also, if you look at the link I sent, you will find that a member of the Visual 
C++ Libraries team at MS is the source for the workaround. They probably know 
what they are doing, present circumstances excepted. 

> That would
> give us some context to estimate the risks of this code executing
> when it's not really needed. 

Hence all the conditions. The problem is *certain* to occur under these 
specific conditions (x64 code on Windows before 7SP1 on a CPU with AVX2 when 
built with VS2013), and under no others, and these conditions flip the switch 
exactly then. 

> Without that, I'd be worried that
> this cure is worse than the disease because it breaks cases that
> weren't broken before.

Isn't that what the buildfarm is (among other things) for?

-- 
Christian Ullrich 


-- 
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] Defaults for replication/backup

[Tom Lane]

Magnus Hagander  writes:
> Yes, these changes will increase some of the default overhead. My argument
> against that is that anybody who actually cares about that overhead is
> going to be tuning their database *anyway*, so they can just change things
> back to the old defaults.

> So, I suggest the following changes to the defaults:

> wal_level=hot_standby
> max_wal_senders=10
> max_replication_slots=10

It would be easier to sell this if we had some numbers for the amount of
overhead it would add for people *not* using the features.  I do not think
I've ever seen, eg, pgbench results with different wal_level and all else
the same.

> And in pg_hba.conf, we enable the replication connections by default for
> the superuser on local/localhost.

Potential security implications?

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] Defaults for replication/backup

[Magnus Hagander]

On Sat, Feb 13, 2016 at 4:52 PM, Tom Lane  wrote:

> Magnus Hagander  writes:
> > Yes, these changes will increase some of the default overhead. My
> argument
> > against that is that anybody who actually cares about that overhead is
> > going to be tuning their database *anyway*, so they can just change
> things
> > back to the old defaults.
>
> > So, I suggest the following changes to the defaults:
>
> > wal_level=hot_standby
> > max_wal_senders=10
> > max_replication_slots=10
>
> It would be easier to sell this if we had some numbers for the amount of
> overhead it would add for people *not* using the features.  I do not think
> I've ever seen, eg, pgbench results with different wal_level and all else
> the same.
>

That's going to be extremely workload dependent. For example, I'd expect
the overhead to be very close to 0 on a pgbench SELECT only benchmark :)

The big thing is, IIRC, that we turn off the optimizations in
min_wal_level. *most* people will see no impact of their regular
application runtime from that, but it might definitely have an effect on
data loads and such. For normal runtime, there should be very close to zero
difference, no?




> > And in pg_hba.conf, we enable the replication connections by default for
> > the superuser on local/localhost.
>
> Potential security implications?
>
>
Since we already allow superuser login from that same target, I don't see
any.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] Defaults for replication/backup

[Tom Lane]

Magnus Hagander  writes:
> On Sat, Feb 13, 2016 at 4:52 PM, Tom Lane  wrote:
>> It would be easier to sell this if we had some numbers for the amount of
>> overhead it would add for people *not* using the features.  I do not think
>> I've ever seen, eg, pgbench results with different wal_level and all else
>> the same.

> That's going to be extremely workload dependent. For example, I'd expect
> the overhead to be very close to 0 on a pgbench SELECT only benchmark :)

> The big thing is, IIRC, that we turn off the optimizations in
> min_wal_level. *most* people will see no impact of their regular
> application runtime from that, but it might definitely have an effect on
> data loads and such. For normal runtime, there should be very close to zero
> difference, no?

I was asking for a demonstration of that, not just handwaving.  Even if
it was measured years ago, I wouldn't assume the comparison would be
the same on current Postgres.

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] Defaults for replication/backup

[Andres Freund]

On 2016-02-13 11:10:58 -0500, Tom Lane wrote:
> Magnus Hagander  writes:
> > The big thing is, IIRC, that we turn off the optimizations in
> > min_wal_level. *most* people will see no impact of their regular
> > application runtime from that, but it might definitely have an effect on
> > data loads and such. For normal runtime, there should be very close to zero
> > difference, no?
> 
> I was asking for a demonstration of that, not just handwaving.  Even if
> it was measured years ago, I wouldn't assume the comparison would be
> the same on current Postgres.

Well, let's look at what the difference between wal_level's are:
1) the (currently broken) optimization of not WAL logging COPY et al,
   for newly created relations.
2) relation AccessExclusiveLocks are WAL logged on >= hot_standby
3) Subtransaction assignment records are generated for >= hot_standby
   after 64 records.
4) checkpoints and bgwriter occasionally generate XLOG_RUNNING_XACTS
   records
5) btreevacuum() and _bt_getbuf() sometimes do additional WAL logging on
   >= hot_standby
6) Once per vacuum we issue a XLOG_HEAP2_CLEANUP_INFO


1) obviously can have performance impact; but only in a relatively
narrow set of cases. I doubt any of the others really play that major a
role.  But I really think minor efficiency differences are besides the
point. Making backups and replication easier has a far bigger positive
impact, for far more users.

- 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] Crash with old Windows on new CPU

[Yury Zhuravlev]

Tom Lane wrote:

Lastly, I'd like to see some discussion of what side effects
"_set_FMA3_enable(0);" has ... I rather doubt that it's really
a magic-elixir-against-crashes-with-no-downsides.  That would
give us some context to estimate the risks of this code executing
when it's not really needed.  Without that, I'd be worried that
this cure is worse than the disease because it breaks cases that
weren't broken befor


I think I have already proposed to drop MSVC support? :)
Too many problems due to of one compiler...

PS Cry from the heart. I work too much with him. Never mind.

--
Yury Zhuravlev
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] Crash with old Windows on new CPU

[Christian Ullrich]

* From: Christian Ullrich

> On February 13, 2016 4:10:34 PM Tom Lane  wrote:
> 
> > Christian Ullrich  writes:

> > Lastly, I'd like to see some discussion of what side effects
> > "_set_FMA3_enable(0);" has ... I rather doubt that it's really
> > a magic-elixir-against-crashes-with-no-downsides.
> 
> It tells the math library (in the CRT, no separate libm on Windows)
> not to use the AVX2-based implementations of log() and possibly
> other functions. AIUI, FMA means "fused multiply-add" and is
> apparently something that increases performance and accuracy in
> transcendental functions.
> 
> I can check the CRT source later today and figure out exactly what
> it does.

OK, it turns out that the CRT source MS ships is not quite as complete as I 
thought it was (up until 2013, at least), so I had a look at the disassembly. 
When the library initializes, it checks whether the CPU supports the FMA 
instructions by looking at a certain bit in the CPUID result. If that is set, 
it sets a flag to use the FMA instructions. Later, in exp(), log(), pow() and 
the trigonometrical functions, it first checks whether that flag is set, and if 
so, uses the AVX-based implementation. If the flag is not set, it falls back to 
an SSE2-based one. So, yes, that function only and specifically disables the 
use of instructions that do not work in the problematic case.

The bug appears to be that it uses all manner of AVX and AVX2 instructions 
based only on the FMA support flag in CPUID, even though AVX2 has its own bit 
there.

To reiterate: The problem occurs because the library only asks the CPU whether 
it is *able* to perform the AVX instructions, but not whether it is *willing* 
to do so. In this particular situation, the former applies but not the latter, 
because the CPU needs OS support (saving the XMM/YMM registers across context 
switches), and the OS has not declared its support for that.

The downside to disabling the AVX implementations is a performance loss 
compared to using it. I ran a microbenchmark (avg(log(x) from 
generate_series(1,1e8))), and the result was that with FMA enabled, it is ~5.5% 
faster than without.

-- 
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] extend pgbench expressions with functions

[Michael Paquier]

On Sun, Feb 14, 2016 at 12:37 AM, Fabien COELHO wrote:
> The two features are highly intermix, so it can only be dependent patches,
> first to add a function infrastructure and probably some support for doubles
> altough it would not be used, then to add doubles & their functions.
>
> A real pain is the documentation, because it means writing a documentation
> with only integer functions, then overwriting it with doubles. This is dumb
> work, really, for the sake of "a cleaner git history", the beauty of it no
> one will ever contemplate...

FWIW, I care a lot about splitting as much as possible patches where
it is possible to have a clean history. So I would be fine to do a
portion of the legwork and extract from this patch something smaller
that adds only functions as a first step, with the minimum set of
functions I mentioned upthread. Robert, Alvaro, Fabien, does that
sound fine to you?
-- 
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] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Robert Haas]

On Sat, Feb 13, 2016 at 1:32 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> Introduce group locking to prevent parallel processes from deadlocking.
>
> I'm fairly unhappy about this patch, because it has introduced a large
> amount of new complexity into the lock manager with effectively no
> documentation.  Yeah, there's several paragraphs added to lmgr/README,
> but they're just a handwavy apologia for the high-level decision to allow
> exclusive locks taken by parallel workers to not conflict.  I see nothing
> whatever explaining how it works, that is any useful docs about the new
> data structures or invariants.  For example, I can't figure out whether
> you have broken the pg_locks display (by causing relevant lock state to
> not get displayed, or get displayed in a useless fashion because one
> can no longer tell which entries block which other entries).  I can't
> even tell for sure if it works at all: the README says only that locks
> taken by members of the same parallel group don't conflict, but the
> implementation looks more like some operations get applied to the group
> leader rather than to followers, which is something completely different.

OK.  I don't know exactly what new documentation needs to be written,
but I'm happy to try to work to improve it.  Obviously, it was clear
enough to me, but that's not saying a lot.  Let me try to answer some
of your questions here via email, and then we can decide what of that
should go into comments, the lmgr README, or someplace else.

First, the overall concept here is that processes can either be a
member of a lock group or a member of no lock group.  The concept of a
lock group is formally separate from the concept of a parallel group
created by a ParallelContext, but it is not clear that there will ever
be any other context in which a lock group will be a good idea.  It is
not impossible to imagine: for example, suppose you had a group of
backends that all had TCP client connections, and those processes all
wanted to ingest data and stuff it in a table but without allowing any
unrelated process to touch the table, say because it was going to be
inconsistent during the operation and until indexes were afterwards
rebuilt.  I don't have any plans to implement anything like that but I
felt it was better to keep the concept of a lock group - which is a
group of processes that cooperate so closely that their locks need not
conflict - from the concept of a parallel context - which is a leader
process that is most likely connected to a user plus a bunch of
ephemeral background workers that aren't.  That way, if somebody later
wants to try to reuse the lock grouping stuff for something else,
nothing will get in the way of that; if not, no harm done, but keeping
the two things decoupled is at least easier to understand, IMHO.

Second, I can review the data structure changes and the associated
invariants.  Each PGPROC has four new members:
lockGroupLeaderIdentifier, lockGroupLeader, lockGroupMembers, and
lockGroupLink.  The first is simply a safety mechanism.  A newly
started parallel worker has to try to join the leader's lock group,
but it has no guarantee that the group leader is still alive by the
time it gets started.  I tried to ensure that the parallel leader dies
after all workers in normal cases, but also that the system could
survive relatively intact if that somehow fails to happen.  This is
one of the precautions against such a scenario: the leader relays its
PGPROC and also its PID to the worker, and the worker fails to join
the lock group unless the given PGPROC still has the same PID.  I
assume that PIDs are not recycled quickly enough for this interlock to
fail.

lockGroupLeader is NULL for processes not involved in parallel query.
When a process wants to cooperate with parallel workers, it becomes a
lock group leader, which means setting this field back to point to its
own PGPROC.  When a parallel worker starts up, it points this field at
the leader, with the above-mentioned interlock.  The lockGroupMembers
field is only used in the leader; it is a list of the workers.  The
lockGroupLink field is used to link the leader and all workers into
the leader's list.  All of these fields are protected by the lock
manager locks; the lock manager lock that protects the fields in a
given PGPROC is chosen by taking pgprocno modulo the number of lock
manager partitions.  This unusual arrangement has a major advantage:
the deadlock detector can count on the fact that no lockGroupLeader
field can change while the deadlock detector is running, because it
knows that it holds all the lock manager locks.  Earlier versions of
the patch protected these fields with the PGPROC's backendLock, but it
was very hard to be confident that this wouldn't lead to deadlock
where one process acquires a lock manager lock and then a backendLock
and another process does the reverse.  Even if that didn't happen, it
was inconvenient not 

Re: [HACKERS] Bug in StartupSUBTRANS

[Jeff Janes]

On Tue, Feb 9, 2016 at 10:33 AM, Simon Riggs  wrote:
> On 9 February 2016 at 18:42, Jeff Janes  wrote:
>>
>> While testing the crash resilience of the recent 2-part-commit
>> improvements, I've run into a problem where sometimes after a crash
>> the recovery process creates zeroed files in pg_subtrans until it
>> exhausts all disk space.
>
>
> Not sure which patch you're talking about there (2-part-commit).

The one here:

commit 978b2f65aa1262eb4ecbf8b3785cb1b9cf4db78e
Author: Simon Riggs 
Date:   Wed Jan 20 18:40:44 2016 -0800

Speedup 2PC by skipping two phase state files in normal path

I thought there was a second commit in the series, but I guess that is
still just proposed.  I've haven't tested that one, just the committed
one.

I've repeated the crash/recovery/wrap-around testing with the proposed
fix in place, and this pre-existing problem seems to be fixed now, and
I have found no other problems.

I've attached a new version, incorporating comments from Tom and Michael.

Cheers,

Jeff


StartupSUB_v2.patch
Description: Binary data

-- 
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] Defaults for replication/backup

[Robert Haas]

On Sat, Feb 13, 2016 at 11:31 AM, Andres Freund  wrote:
> On 2016-02-13 11:10:58 -0500, Tom Lane wrote:
>> Magnus Hagander  writes:
>> > The big thing is, IIRC, that we turn off the optimizations in
>> > min_wal_level. *most* people will see no impact of their regular
>> > application runtime from that, but it might definitely have an effect on
>> > data loads and such. For normal runtime, there should be very close to zero
>> > difference, no?
>>
>> I was asking for a demonstration of that, not just handwaving.  Even if
>> it was measured years ago, I wouldn't assume the comparison would be
>> the same on current Postgres.
>
> Well, let's look at what the difference between wal_level's are:
> 1) the (currently broken) optimization of not WAL logging COPY et al,
>for newly created relations.
> 2) relation AccessExclusiveLocks are WAL logged on >= hot_standby
> 3) Subtransaction assignment records are generated for >= hot_standby
>after 64 records.
> 4) checkpoints and bgwriter occasionally generate XLOG_RUNNING_XACTS
>records
> 5) btreevacuum() and _bt_getbuf() sometimes do additional WAL logging on
>>= hot_standby
> 6) Once per vacuum we issue a XLOG_HEAP2_CLEANUP_INFO
>
>
> 1) obviously can have performance impact; but only in a relatively
> narrow set of cases. I doubt any of the others really play that major a
> role.  But I really think minor efficiency differences are besides the
> point. Making backups and replication easier has a far bigger positive
> impact, for far more users.

I think that there are definitely people for whom #1 is an issue, but
maybe that's not a sufficient reason not to change the default.

As a thought experiment, how about teaching initdb how to tailor the
configuration to a couple of common scenarios via some new flag?  I'll
call it --setup although that's probably not best.

--setup=replication   --- Preconfigure for replication.
--setup=standalone  --- Preconfigure for standalone mode.
--setup=ephemeral  --- Preconfigure for an ephemeral instance that
doesn't need durability because we'll blow it up soon.

Whichever mode we make the default, I think this kind of thing would
make life easier for users.

-- 
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] extend pgbench expressions with functions

[Robert Haas]

On Sat, Feb 13, 2016 at 6:19 PM, Michael Paquier
 wrote:
> On Sun, Feb 14, 2016 at 12:37 AM, Fabien COELHO wrote:
>> The two features are highly intermix, so it can only be dependent patches,
>> first to add a function infrastructure and probably some support for doubles
>> altough it would not be used, then to add doubles & their functions.
>>
>> A real pain is the documentation, because it means writing a documentation
>> with only integer functions, then overwriting it with doubles. This is dumb
>> work, really, for the sake of "a cleaner git history", the beauty of it no
>> one will ever contemplate...
>
> FWIW, I care a lot about splitting as much as possible patches where
> it is possible to have a clean history. So I would be fine to do a
> portion of the legwork and extract from this patch something smaller
> that adds only functions as a first step, with the minimum set of
> functions I mentioned upthread. Robert, Alvaro, Fabien, does that
> sound fine to you?

I'd be delighted.  I would really like to get this feature in, but I'm
not going to do it if it requires an unreasonable amount of work on my
part - and what you propose would help a lot.

-- 
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] extend pgbench expressions with functions

[Robert Haas]

On Sat, Feb 13, 2016 at 10:37 AM, Fabien COELHO  wrote:
> A real pain is the documentation, because it means writing a documentation
> with only integer functions, then overwriting it with doubles. This is dumb
> work, really, for the sake of "a cleaner git history", the beauty of it no
> one will ever contemplate...

You know, you make comments like this pretty much every time anybody
suggests that you should change anything in any patch you write.  It
doesn't matter whether the change is suggested by Heikki, or by
Michael, or by Andres, or by me.  You pretty much always come back and
say something that amounts to "changing the patch I already wrote is a
waste of time".  That gets a little disheartening after a while.  This
community's procedure is that patches have to be reviewed and reach
consensus in order to get committed, and in my opinion that is
generally not "dumb" but rather something that enhances the end
result.  I value your contributions to the community and I hope you
will continue making them, but I don't like it when people call my
ideas dumb.

-- 
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] Way to check whether a particular block is on the shared_buffer?

[Robert Haas]

On Sat, Feb 13, 2016 at 7:29 AM, Kouhei Kaigai  wrote:
>> I suppose there's no theoretical reason why the buffer couldn't go
>> from all-visible to not-all-visible and back to all-visible again all
>> during the time you are copying it.
>>
> The backend process that is copying the data to GPU has a transaction
> in-progress (= not committed). Is it possible to get the updated buffer
> page back to the all-visible state again?
> I expect that in-progress transactions works as a blocker for backing
> to all-visible. Right?

Yeah, probably.

-- 
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] extend pgbench expressions with functions

[Fabien COELHO]

Hello Robert,


You know, you make comments like this pretty much every time anybody
suggests that you should change anything in any patch you write.


Well, not everytime...

I'm tired of doing and undoing things: I do a limited A because I'm 
cautious not to spend too much time that would go down the drain. Then I'm 
told "do A+B" to show that A is worthwhile. Then I'm told "you should not 
have done B with A, but submit A for the infrastructure and then B for the 
additional features", which was precisely my initial intent...


So this is really the going forward and backwards because of the process 
that makes me write these remarks.



[...] I don't like it when people call my ideas dumb.


Your idea is not dumb, I'm sorry if I have implied that, and I apologise. 
I like a clean history as much as everyone else.


Having to unmix features and documentations is still "dumb work", even if 
the purpose is not dumb, I stand by this word for this part.


--
Fabien.


--
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] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Craig Ringer]

On 14 February 2016 at 08:05, Robert Haas  wrote:


> First, the overall concept here is that processes can either be a
> member of a lock group or a member of no lock group.  The concept of a
> lock group is formally separate from the concept of a parallel group
> created by a ParallelContext, but it is not clear that there will ever
> be any other context in which a lock group will be a good idea.  It is
> not impossible to imagine: for example, suppose you had a group of
> backends that all had TCP client connections, and those processes all
> wanted to ingest data and stuff it in a table but without allowing any
> unrelated process to touch the table, say because it was going to be
> inconsistent during the operation and until indexes were afterwards
> rebuilt.


The case that comes to mind for me is in logical decoding, for decoding
prepared xacts. Being able to make the prepared xact a member of a "lock
group" along with the decoding session's xact may provide a solution to the
locking-related challenges there.

I haven't looked closely at what's involved in the decoding prepared xact
locking issues yet, just an idea.

To do this it'd have to be possible to add an existing session/xact to a
lock group (or make it the leader of a new lock group then join that
group). Do you think that's practical with your design?


> I don't have any plans to implement anything like that but I
> felt it was better to keep the concept of a lock group - which is a
> group of processes that cooperate so closely that their locks need not
> conflict - from the concept of a parallel context - which is a leader
> process that is most likely connected to a user plus a bunch of
> ephemeral background workers that aren't.  That way, if somebody later
> wants to try to reuse the lock grouping stuff for something else,
> nothing will get in the way of that; if not, no harm done, but keeping
> the two things decoupled is at least easier to understand, IMHO.
>

Yeah, strong +1

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: [HACKERS] extend pgbench expressions with functions

[Fabien COELHO]

So I would be fine to do a portion of the legwork and extract from this 
patch something smaller that adds only functions as a first step, with 
the minimum set of functions I mentioned upthread. Robert, Alvaro, 
Fabien, does that sound fine to you?


Thanks, but this is my c*, I have a few hours of travelling this evening, 
I'll do it then.


I'll be happy if you do the review of the resulting split.

--
Fabien.


--
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] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Stephen Frost]

* Craig Ringer (cr...@2ndquadrant.com) wrote:
> On 14 February 2016 at 08:05, Robert Haas  wrote:
> > First, the overall concept here is that processes can either be a
> > member of a lock group or a member of no lock group.  The concept of a
> > lock group is formally separate from the concept of a parallel group
> > created by a ParallelContext, but it is not clear that there will ever
> > be any other context in which a lock group will be a good idea.  It is
> > not impossible to imagine: for example, suppose you had a group of
> > backends that all had TCP client connections, and those processes all
> > wanted to ingest data and stuff it in a table but without allowing any
> > unrelated process to touch the table, say because it was going to be
> > inconsistent during the operation and until indexes were afterwards
> > rebuilt.
> 
> The case that comes to mind for me is in logical decoding, for decoding
> prepared xacts. Being able to make the prepared xact a member of a "lock
> group" along with the decoding session's xact may provide a solution to the
> locking-related challenges there.

I was thinking this would be the way to address the current issues with
parallel pg_dump and having a shared snpashot.

> I haven't looked closely at what's involved in the decoding prepared xact
> locking issues yet, just an idea.

Yeah, don't have much more than it being an idea to use this for the
pg_dump case.  I do know that's a case which has been brought up a
couple of times before.

> To do this it'd have to be possible to add an existing session/xact to a
> lock group (or make it the leader of a new lock group then join that
> group). Do you think that's practical with your design?

Seems like the same question applies to the pg_dump case.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Crash with old Windows on new CPU

[Craig Ringer]

On 13 February 2016 at 23:45, Christian Ullrich 
wrote:


>
> > Maybe that's actually
> > what's needed, but it sure looks fishy.  And what connection does the
> > build toolchain version have to the runtime environment anyway?
>
> The CRT version is tied to the compiler version. It has mainly to do with
> matching allocators.
>

In UNIX terms, the compiler and libc are tightly coupled on Windows and
each executable or shared library links to the C library provided by the
compiler that built the DLL or EXE. It's normal to link a program with
multiple C runtime libraries if that program is composed of one or more
DLLs that were each compiled by a different toolchain and therefore linked
to a different C runtime ... which is common in the Windows binary
distribution model.

"msvcrt.dll" is the best-known C runtime, the one bundled in Windows. It's
ancient and buggy and retained only for backward compatibility. mingw has
to pile hacks upon hacks around it to use it.

Microsoft has been bundling the C runtime libraries with the compiler since
at least Visual Studio 7. Application installers are expected to silently
run an installer for the C runtime library before installing the
application. They do not use the C runtime library from the operating
system.

You might have seen "Visual Studio Redistributable" packages installed by
various app installers. That's what they are - they're making sure the
specific C runtime they are built against is installed.

It's a bit like, on UNIX, each shared library expecting to have a different
libc. One expects libc-2.20.so, one expects libc-2.19.so, etc. Each with
linkage private to the shared library that loaded it. (We have this to a
lesser degree with libgcc, but everyone uses one version of gcc for
building everything on a given system so in practice it makes no
difference.)

This is why applications on Windows must be very careful not to malloc()
memory in one DLL and free() it in another, etc. They're effectively
leaking it and freeing unallocated memory at the same time because the two
C libraries have separate memory management structures.

It lets applications be composed of binary-distributed components from
different vendors, using different toolchains. Want to build new parts of
an app with VS 2017 while compiling your legacy components with VS 7,
linking with a libabc built using mingw32 by a vendor who won't show you
the source, and linking a libxyz built with an ancient version of Borland C
from sources you lost 5 years ago? You can do that.

I don't pretend it's a good idea, but it does have advantages sometimes.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: [HACKERS] proposal: function parse_ident

[Pavel Stehule]

Hi Jim

2016-02-11 8:27 GMT+01:00 Pavel Stehule 
>
>
> ok
>
>>
>> Also added test for invalid characters.
>>
>> I think "strict" would be more in line with other uses in code. There are
>> currently no other occurrences of 'strictmode' in the code. There are loads
>> of references to 'strict', but I didn't go through all of them to see if
>> any were used as externally visible function parameter names.
>>
>
> I am sorry, I don't understand to this point. You unlike the name of
> parameter "strictmode" ? Have you any proposal? Maybe "restrictive" ?
>

Please, can you explain this point?

Regards

Pavel


>
>
>>
>>

Re: [HACKERS] extend pgbench expressions with functions

[Michael Paquier]

On Sun, Feb 14, 2016 at 10:05 AM, Robert Haas  wrote:
> On Sat, Feb 13, 2016 at 6:19 PM, Michael Paquier
>  wrote:
>> On Sun, Feb 14, 2016 at 12:37 AM, Fabien COELHO wrote:
>>> The two features are highly intermix, so it can only be dependent patches,
>>> first to add a function infrastructure and probably some support for doubles
>>> altough it would not be used, then to add doubles & their functions.
>>>
>>> A real pain is the documentation, because it means writing a documentation
>>> with only integer functions, then overwriting it with doubles. This is dumb
>>> work, really, for the sake of "a cleaner git history", the beauty of it no
>>> one will ever contemplate...
>>
>> FWIW, I care a lot about splitting as much as possible patches where
>> it is possible to have a clean history. So I would be fine to do a
>> portion of the legwork and extract from this patch something smaller
>> that adds only functions as a first step, with the minimum set of
>> functions I mentioned upthread. Robert, Alvaro, Fabien, does that
>> sound fine to you?
>
> I'd be delighted.  I would really like to get this feature in, but I'm
> not going to do it if it requires an unreasonable amount of work on my
> part - and what you propose would help a lot.

OK, I'll see about producing a patch then for this basic
infrastructure, with the rest built on top of it as a secondary patch.
-- 
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] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Robert Haas]

On Sat, Feb 13, 2016 at 9:20 PM, Craig Ringer  wrote:
> The case that comes to mind for me is in logical decoding, for decoding
> prepared xacts. Being able to make the prepared xact a member of a "lock
> group" along with the decoding session's xact may provide a solution to the
> locking-related challenges there.
>
> I haven't looked closely at what's involved in the decoding prepared xact
> locking issues yet, just an idea.
>
> To do this it'd have to be possible to add an existing session/xact to a
> lock group (or make it the leader of a new lock group then join that group).
> Do you think that's practical with your design?

I doubt it.  It's only safe to join a locking group if you don't yet
hold any heavyweight locks.  I'm not going to say it'd be impossible
to lift that restriction, but it'd be pretty complex, because doing so
could either create or remove deadlocks that didn't exist before.  For
example, suppose A wanting AccessExclusiveLock waits for B wanting
AccessExclusvieLock waits for C holding AccessShareLock.  Then, C
joins A's lock group.  If A's lock request can't be granted
immediately - say D also holds AccessShareLock on the object - this is
a deadlock.   Moreover, C can't detect the deadlock in the normal
course of things because C is not waiting.  Sorting this out does not
sound simple.

It could possibly work if the decoding transaction holds no locks at
all, joins the prepared xact's locking group, does stuff, and then,
when it again reaches a point where it holds no locks, leaves the lock
group.  I wonder, though, what happens if you deadlock.  The decoding
transaction get killed, but you can't kill the prepared transaction,
so any locks it held would be retained.  Maybe that's OK, but I have a
sneaking suspicion there might be situations where we kill the
decoding transaction without resolving the deadlock.  Sharing locks
with a prepared transaction is not really what this was designed for.

I don't really understand what problem you are trying to solve here,
but I suspect there is a better solution than group locking.  The
thing is, in the normal course of events, heavyweight locking prevents
a lot of bad stuff from happening.  When you become a member of a lock
group, you're on your own recognizance to prevent that bad stuff.  The
parallel code does that (or hopefully does that, anyway) by imposing
severe restrictions on what you can do while in parallel mode; those
restrictions include "no writes whatsoever" and "no DDL".  If you
wanted to allow either of those things, you would need to think very,
very carefully about that, and then if you decided that it was going
to be safe, you'd need to think carefully about it a second time.

As I mentioned to Simon on another thread a while back, Thomas Munro
is working on a hash table that uses dynamic shared memory, and as
part of that work, he is taking the allocator work that I did a year
or two ago and turning that into a full-fledged allocator for dynamic
shared memory.  Once we have a real allocator and a real hash table
for DSM, I believe we'll be able to solve some of the problems that
currently require that parallel query - and probably anything that
uses group locking - be strictly read-only.  For example, we can use
that infrastructure to store a combo CID hash table that can grow
arbitrarily in a data structure that all cooperating processes can
share.  Unfortunately, it does not look like that work will be ready
in time to be included in PostgreSQL 9.6.  I think he will be in a
position to submit it in time for PostgreSQL 9.7, though.

>> I don't have any plans to implement anything like that but I
>> felt it was better to keep the concept of a lock group - which is a
>> group of processes that cooperate so closely that their locks need not
>> conflict - from the concept of a parallel context - which is a leader
>> process that is most likely connected to a user plus a bunch of
>> ephemeral background workers that aren't.  That way, if somebody later
>> wants to try to reuse the lock grouping stuff for something else,
>> nothing will get in the way of that; if not, no harm done, but keeping
>> the two things decoupled is at least easier to understand, IMHO.
>
> Yeah, strong +1

Thanks.

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