Re: [HACKERS] Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file

[Amit Kapila]

On Tue, Jun 9, 2015 at 10:56 AM, Fujii Masao  wrote:
>
> On Tue, Jun 9, 2015 at 1:04 PM, Amit Kapila 
wrote:
> > On Tue, Jun 9, 2015 at 9:09 AM, Fujii Masao 
wrote:
> >>
> >> On Tue, May 12, 2015 at 10:42 PM, Andrew Dunstan 
> >> wrote:
> >> > Map basebackup tablespaces using a tablespace_map file
> >> >
> >> > Windows can't reliably restore symbolic links from a tar format, so
> >> > instead during backup start we create a tablespace_map file, which is
> >> > used by the restoring postgres to create the correct links in
pg_tblspc.
> >> > The backup protocol also now has an option to request this file to be
> >> > included in the backup stream, and this is used by pg_basebackup when
> >> > operating in tar mode.
> >> >
> >> > This is done on all platforms, not just Windows.
> >> >
> >> > This means that pg_basebackup will not not work in tar mode against
9.4
> >> > and older servers, as this protocol option isn't implemented there.
> >>
> >> While I was performing the recovery-related tests, I found that there
was
> >> the case where $PGDATA/tablespace_map was not renamed to *.old at the
> >> recovery.
> >> Is this harmless and intentional?
> >
> > There shouldn't be any problem because we tablespace_map file
> > only if backup file is present.
> >
> >> Sorry if this has been already
> >> discussed so far.
> >>
> >> The steps to reproduce the problem is:
> >>
> >> 1. start base backup, i.e., call pg_start_backup().
> >> 2. repeat some write transactions and checkpoints until the WAL file
> >> containing
> >> the checkpoint record that backup_label indicates will be removed.
> >> 3. killall -9 postgres
> >> 4. start the server and a crash recovery.
> >>
> >> At this time, the crash recovery fails with the following error
messages.
> >> 2015-06-09 12:26:54 JST FATAL:  could not locate required checkpoint
> >> record
> >> 2015-06-09 12:26:54 JST HINT:  If you are not restoring from a backup,
> >> try removing the file ".../backup_label".
> >>
> >> 5. according to the above hint message, remove $PGDATA/backup_label
> >> and restart a crash recovery
> >>
> >> Then you can see that tablespace_map remains in $PGDATA after the
recovery
> >> ends.
> >>
> >
> > The basic idea is that tablespace_map file will be used in case we
> > have to restore from a backup which contains tablespaces.  So
> > I think if user is manually removing backup_label, there is no
> > meaning of tablespace_map, so that should also be removed. One
> > way to address this is modify the Hint message suggesting that
> > remove tablespace_map if present.
> >
> > Current :
> > If you are not restoring from a backup, try removing the file
> > \"%s/backup_label\
> > Modify it to:
> > If you are not restoring from a backup, try removing the file
> > \"%s/backup_label\
> > and, if present \"%s/tablespace_map\.
>
> Or what about removing tablespace_map file at the beginning of recovery
> whenever backup_label doesn't exist?

Yes, thats another way, but is it safe to assume that user won't need
that file, I mean in the valid scenario (where both backup_label and
tablespace_map are present and usable) also, we rename them to
*.old rather than deleting it.

Yet another way could be we return error if tablespace_map is
present whenever backup_label doesn't exist?

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

Re: [HACKERS] Information of pg_stat_ssl visible to all users

[Magnus Hagander]

On Jun 9, 2015 6:00 AM, "Michael Paquier"  wrote:
>
> Hi all,
>
> I should have noticed that before, but it happens that pg_stat_ssl
> leaks information about the SSL status of all the users connected to a
> server. Let's imagine for example:
> 1) Session 1 connected through SSL with a superuser:
> =# create role toto login;
> CREATE ROLE
> =# select * from pg_stat_ssl;
>   pid  | ssl | version |   cipher| bits |
> compression | clientdn
>
---+-+-+-+--+-+--
>  33348 | t   | TLSv1.2 | ECDHE-RSA-AES256-GCM-SHA384 |  256 | t
 |
> (1 row)
> 2) New session 2 with previously created user:
> => select * from pg_stat_ssl;
>   pid  | ssl | version |   cipher| bits |
> compression | clientdn
>
---+-+-+-+--+-+--
>  33348 | t   | TLSv1.2 | ECDHE-RSA-AES256-GCM-SHA384 |  256 | t
 |
>  33367 | t   | TLSv1.2 | ECDHE-RSA-AES256-GCM-SHA384 |  256 | t
 |
> (2 rows)
>
> Attached is a patch to mask those values to users that should not have
> access to it, similarly to the other fields of pg_stat_activity.

I don't have the thread around right now (on phone), but didn't we discuss
this back around the original submission and decide that this was wanted
behavior?

What actual sensitive data is leaked? If knowing the cipher type makes it
easier to hack you have a broken cipher, don't you?

/Magnus

Re: [HACKERS] Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file

[Fujii Masao]

On Tue, Jun 9, 2015 at 1:04 PM, Amit Kapila  wrote:
> On Tue, Jun 9, 2015 at 9:09 AM, Fujii Masao  wrote:
>>
>> On Tue, May 12, 2015 at 10:42 PM, Andrew Dunstan 
>> wrote:
>> > Map basebackup tablespaces using a tablespace_map file
>> >
>> > Windows can't reliably restore symbolic links from a tar format, so
>> > instead during backup start we create a tablespace_map file, which is
>> > used by the restoring postgres to create the correct links in pg_tblspc.
>> > The backup protocol also now has an option to request this file to be
>> > included in the backup stream, and this is used by pg_basebackup when
>> > operating in tar mode.
>> >
>> > This is done on all platforms, not just Windows.
>> >
>> > This means that pg_basebackup will not not work in tar mode against 9.4
>> > and older servers, as this protocol option isn't implemented there.
>>
>> While I was performing the recovery-related tests, I found that there was
>> the case where $PGDATA/tablespace_map was not renamed to *.old at the
>> recovery.
>> Is this harmless and intentional?
>
> There shouldn't be any problem because we tablespace_map file
> only if backup file is present.
>
>> Sorry if this has been already
>> discussed so far.
>>
>> The steps to reproduce the problem is:
>>
>> 1. start base backup, i.e., call pg_start_backup().
>> 2. repeat some write transactions and checkpoints until the WAL file
>> containing
>> the checkpoint record that backup_label indicates will be removed.
>> 3. killall -9 postgres
>> 4. start the server and a crash recovery.
>>
>> At this time, the crash recovery fails with the following error messages.
>> 2015-06-09 12:26:54 JST FATAL:  could not locate required checkpoint
>> record
>> 2015-06-09 12:26:54 JST HINT:  If you are not restoring from a backup,
>> try removing the file ".../backup_label".
>>
>> 5. according to the above hint message, remove $PGDATA/backup_label
>> and restart a crash recovery
>>
>> Then you can see that tablespace_map remains in $PGDATA after the recovery
>> ends.
>>
>
> The basic idea is that tablespace_map file will be used in case we
> have to restore from a backup which contains tablespaces.  So
> I think if user is manually removing backup_label, there is no
> meaning of tablespace_map, so that should also be removed. One
> way to address this is modify the Hint message suggesting that
> remove tablespace_map if present.
>
> Current :
> If you are not restoring from a backup, try removing the file
> \"%s/backup_label\
> Modify it to:
> If you are not restoring from a backup, try removing the file
> \"%s/backup_label\
> and, if present \"%s/tablespace_map\.

Or what about removing tablespace_map file at the beginning of recovery
whenever backup_label doesn't exist? I'd like to avoid making a user do
such manual operation if possible.

Regards,

-- 
Fujii Masao


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

[HACKERS] Useless mention of RMGRDESCSOURCES in src/bin/pg_rewind/Makefile

[Michael Paquier]

Hi all,

pg_rewind's Makefile uses RMGRDESCSOURCES:
EXTRA_CLEAN = $(RMGRDESCSOURCES) xlogreader.c
However this variable is defined only in the Makefile of pg_xlogdump
so it has no utility in this case.
Attached is a cleanup patch.
Regards,
-- 
Michael
diff --git a/src/bin/pg_rewind/Makefile b/src/bin/pg_rewind/Makefile
index 98213c4..7d60715 100644
--- a/src/bin/pg_rewind/Makefile
+++ b/src/bin/pg_rewind/Makefile
@@ -24,7 +24,7 @@ OBJS	= pg_rewind.o parsexlog.o xlogreader.o datapagemap.o timeline.o \
 	fetch.o file_ops.o copy_fetch.o libpq_fetch.o filemap.o logging.o \
 	$(WIN32RES)
 
-EXTRA_CLEAN = $(RMGRDESCSOURCES) xlogreader.c
+EXTRA_CLEAN = xlogreader.c
 
 all: pg_rewind
 

-- 
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: Map basebackup tablespaces using a tablespace_map file

[Amit Kapila]

On Tue, Jun 9, 2015 at 9:09 AM, Fujii Masao  wrote:
>
> On Tue, May 12, 2015 at 10:42 PM, Andrew Dunstan 
wrote:
> > Map basebackup tablespaces using a tablespace_map file
> >
> > Windows can't reliably restore symbolic links from a tar format, so
> > instead during backup start we create a tablespace_map file, which is
> > used by the restoring postgres to create the correct links in pg_tblspc.
> > The backup protocol also now has an option to request this file to be
> > included in the backup stream, and this is used by pg_basebackup when
> > operating in tar mode.
> >
> > This is done on all platforms, not just Windows.
> >
> > This means that pg_basebackup will not not work in tar mode against 9.4
> > and older servers, as this protocol option isn't implemented there.
>
> While I was performing the recovery-related tests, I found that there was
> the case where $PGDATA/tablespace_map was not renamed to *.old at the
recovery.
> Is this harmless and intentional?

There shouldn't be any problem because we tablespace_map file
only if backup file is present.

> Sorry if this has been already
> discussed so far.
>
> The steps to reproduce the problem is:
>
> 1. start base backup, i.e., call pg_start_backup().
> 2. repeat some write transactions and checkpoints until the WAL file
containing
> the checkpoint record that backup_label indicates will be removed.
> 3. killall -9 postgres
> 4. start the server and a crash recovery.
>
> At this time, the crash recovery fails with the following error messages.
> 2015-06-09 12:26:54 JST FATAL:  could not locate required checkpoint
record
> 2015-06-09 12:26:54 JST HINT:  If you are not restoring from a backup,
> try removing the file ".../backup_label".
>
> 5. according to the above hint message, remove $PGDATA/backup_label
> and restart a crash recovery
>
> Then you can see that tablespace_map remains in $PGDATA after the
recovery ends.
>

The basic idea is that tablespace_map file will be used in case we
have to restore from a backup which contains tablespaces.  So
I think if user is manually removing backup_label, there is no
meaning of tablespace_map, so that should also be removed. One
way to address this is modify the Hint message suggesting that
remove tablespace_map if present.

Current :
If you are not restoring from a backup, try removing the file
\"%s/backup_label\
Modify it to:
If you are not restoring from a backup, try removing the file
\"%s/backup_label\
and, if present \"%s/tablespace_map\.


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

[HACKERS] Information of pg_stat_ssl visible to all users

[Michael Paquier]

Hi all,

I should have noticed that before, but it happens that pg_stat_ssl
leaks information about the SSL status of all the users connected to a
server. Let's imagine for example:
1) Session 1 connected through SSL with a superuser:
=# create role toto login;
CREATE ROLE
=# select * from pg_stat_ssl;
  pid  | ssl | version |   cipher| bits |
compression | clientdn
---+-+-+-+--+-+--
 33348 | t   | TLSv1.2 | ECDHE-RSA-AES256-GCM-SHA384 |  256 | t   |
(1 row)
2) New session 2 with previously created user:
=> select * from pg_stat_ssl;
  pid  | ssl | version |   cipher| bits |
compression | clientdn
---+-+-+-+--+-+--
 33348 | t   | TLSv1.2 | ECDHE-RSA-AES256-GCM-SHA384 |  256 | t   |
 33367 | t   | TLSv1.2 | ECDHE-RSA-AES256-GCM-SHA384 |  256 | t   |
(2 rows)

Attached is a patch to mask those values to users that should not have
access to it, similarly to the other fields of pg_stat_activity.
Regards,
-- 
Michael
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index f7c9bf6..159860b 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -626,21 +626,6 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 		else
 			nulls[15] = true;
 
-		if (beentry->st_ssl)
-		{
-			values[16] = BoolGetDatum(true);	/* ssl */
-			values[17] = CStringGetTextDatum(beentry->st_sslstatus->ssl_version);
-			values[18] = CStringGetTextDatum(beentry->st_sslstatus->ssl_cipher);
-			values[19] = Int32GetDatum(beentry->st_sslstatus->ssl_bits);
-			values[20] = BoolGetDatum(beentry->st_sslstatus->ssl_compression);
-			values[21] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn);
-		}
-		else
-		{
-			values[16] = BoolGetDatum(false);	/* ssl */
-			nulls[17] = nulls[18] = nulls[19] = nulls[20] = nulls[21] = true;
-		}
-
 		/* Values only available to role member */
 		if (has_privs_of_role(GetUserId(), beentry->st_userid))
 		{
@@ -761,6 +746,22 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 	nulls[13] = true;
 }
 			}
+
+			/* ssl information */
+			if (beentry->st_ssl)
+			{
+values[16] = BoolGetDatum(true);	/* ssl */
+values[17] = CStringGetTextDatum(beentry->st_sslstatus->ssl_version);
+values[18] = CStringGetTextDatum(beentry->st_sslstatus->ssl_cipher);
+values[19] = Int32GetDatum(beentry->st_sslstatus->ssl_bits);
+values[20] = BoolGetDatum(beentry->st_sslstatus->ssl_compression);
+values[21] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn);
+			}
+			else
+			{
+values[16] = BoolGetDatum(false);	/* ssl */
+nulls[17] = nulls[18] = nulls[19] = nulls[20] = nulls[21] = true;
+			}
 		}
 		else
 		{
@@ -775,6 +776,13 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 			nulls[11] = true;
 			nulls[12] = true;
 			nulls[13] = true;
+			/* ssl information */
+			nulls[16] = true;
+			nulls[17] = true;
+			nulls[18] = true;
+			nulls[19] = true;
+			nulls[20] = true;
+			nulls[21] = true;
 		}
 
 		tuplestore_putvalues(tupstore, tupdesc, values, nulls);

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

[HACKERS] Aggregate Supporting Functions

[David Rowley]

I believe this is an idea that's been discussed before, but I'm not exactly
sure where that happened:

Overview:

The idea is that we skip a major chunk of processing in situations like:

SELECT avg(x),sum(x),count(x) FROM bigtable;

Because avg(x) already technically knows what the values of sum(x) and
count(x) are.

The performance improvement of this particular case is as follows:

create table bigtable as select
generate_series(1,100)::numeric as x; vacuum bigtable;

SELECT avg(x),sum(x),count(x) FROM bigtable; -- Query 1

Time: 390.325 ms
Time: 392.297 ms
Time: 400.790 ms

SELECT avg(x) FROM bigtable; -- Query 2

Time: 219.700 ms
Time: 215.285 ms
Time: 233.691 ms

With the implementation I'm proposing below, I believe that query 1 should
perform almost as well as query 2. The only extra CPU work that would be
done would be some extra checks during planning, and the calling of 2
simple new final functions which will extract the count(x) and sum(x) from
the avg transition's state.

Purpose of this Email:

For technical review of proposed implementation.

Implementation:

1. Add a new boolean column pg_aggregate named hassuppagg which will be set
to true if the aggregate supports other aggregates. For example avg(int)
will support count(int) and sum(int)
2. Add new system table named pg_aggregate_support (Or some better shorter
name)

This system table will be defined as follows:
aspfnoid regproc,
aspfnsupported regproc,
aspfinalfn regproc,
primary key (aspfnoid, aspfnsupported)

Where in the above example aspfnoid will be avg(int) and 2 rows will exist.
1 with count(int) in aspfnsupported, and one with sum(int) in the
aspfnsupported column. aspfinalfn will be a new final function which
extracts the required portion of the avg's aggregate state.

3. Add logic in the planner to look for look for supporting cases. With
logic something along the lines of:

  a. Does the query have any aggregates? If not -> return;
  b. Does the query have more than 1 aggregate? If not -> return;
  c. Does the at least one of the aggregates have hassuppagg set to true?
if not -> return;
  d. Analyze aggregates to eliminate aggregates that are covered by another
aggregate. We should use the aggregate which eliminates the most other
aggregates*

* For example stddev(x) will support avg(x), sum(x) and count(x) so a query
such as select stddev(x), avg(x), sum(x), count(x) will eliminate avg(x),
sum(x), count(x) as stddev(x) supports 3, avg(x) only supports 2, so will
have to be eliminated.

Concerns:

I'm a little bit concerned that someone will one day report that:

SELECT avg(x), sum(x), count(x) from bigtable;

Is faster than:

SELECT sum(x), count(x) from bigtable;

Of course, this will be just because we've made case 1 faster, NOT because
we've slowed down case 2.
I can't immediately think of a way to fix that without risking slowing
down: select count(x) from bigtable;

CREATE AGGREGATE Syntax:

To allow users to implement aggregates which take advantage of this we'd
better also expand the CREATE AGGREGATE syntax.

I've not given this a huge amount of thought. The only thing I've come up
with so far is;

CREATE AGGREGATE avg(bigint)
(FINALFUNC = avgfinal)
SUPPORTS (count(bigint) = int8_avg_countfn, sum(bigint) = int8_avg_sumfn);


Can anyone think of anything that I've not accounted for before I go off
and work on this?

Regards

David Rowley

"The research leading to these results has received funding from the
European Union’s
Seventh Framework Programme (FP7/2007-2015) under grant agreement n° 318633"

--
 David Rowley   http://www.2ndQuadrant.com/

 PostgreSQL Development, 24x7 Support, Training & Services

Re: [HACKERS] Memory leak with XLogFileCopy since de768844 (WAL file with .partial)

[Fujii Masao]

On Tue, Jun 9, 2015 at 6:25 AM, Heikki Linnakangas  wrote:
> On 06/08/2015 09:04 PM, Fujii Masao wrote:
>>
>> On Mon, Jun 8, 2015 at 11:52 AM, Michael Paquier
>>  wrote:
>>>
>>> On Fri, Jun 5, 2015 at 10:45 PM, Fujii Masao wrote:

 Why don't we call InstallXLogFileSegment() at the end of XLogFileCopy()?
 If we do that, the risk of memory leak you're worried will disappear at
 all.
>>>
>>>
>>> Yes, that looks fine, XLogFileCopy() would copy to a temporary file,
>>> then install it definitely. Updated patch attached.
>>
>>
>> Thanks for updating the patch! Looks good to me. Applied.
>
>
> XLogFileCopy() used to call InstallXLogFileSegment(), until I refactored
> that in commit de7688442f5aaa03da60416a6aa3474738718803. That commit added
> another caller of XLogFileCopy(), which didn't want to install the segment.
> However, I later partially reverted that patch in commit
> 7cbee7c0a1db668c60c020a3fd1e3234daa562a9, and those changes to
> XLogFileCopy() were not really needed anymore. I decided not to revert those
> changes at that point because that refactoring seemed sensible anyway. See
> my email at http://www.postgresql.org/message-id/555dd101.7080...@iki.fi
> about that.
>
> I'm still not sure if I should've just reverted that refactoring, to make
> XLogFileCopy() look the same in master and back-branches, which makes
> back-patching easier, or keep the refactoring, because it makes the code
> slightly nicer. But the current situation is the worst of both worlds: the
> interface of XLogFileCopy() is no better than it used to be, but it's
> different enough to cause merge conflicts. At this point, it's probably best
> to revert the code to look the same as in 9.4.

I'm not sure how much it's really nicer to keep the refactoring,
but I agree that it's better to make the code look same to make
the back-patching easier.

Regards,

-- 
Fujii Masao


-- 
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: Map basebackup tablespaces using a tablespace_map file

[Fujii Masao]

On Tue, May 12, 2015 at 10:42 PM, Andrew Dunstan  wrote:
> Map basebackup tablespaces using a tablespace_map file
>
> Windows can't reliably restore symbolic links from a tar format, so
> instead during backup start we create a tablespace_map file, which is
> used by the restoring postgres to create the correct links in pg_tblspc.
> The backup protocol also now has an option to request this file to be
> included in the backup stream, and this is used by pg_basebackup when
> operating in tar mode.
>
> This is done on all platforms, not just Windows.
>
> This means that pg_basebackup will not not work in tar mode against 9.4
> and older servers, as this protocol option isn't implemented there.

While I was performing the recovery-related tests, I found that there was
the case where $PGDATA/tablespace_map was not renamed to *.old at the recovery.
Is this harmless and intentional? Sorry if this has been already
discussed so far.

The steps to reproduce the problem is:

1. start base backup, i.e., call pg_start_backup().
2. repeat some write transactions and checkpoints until the WAL file containing
the checkpoint record that backup_label indicates will be removed.
3. killall -9 postgres
4. start the server and a crash recovery.

At this time, the crash recovery fails with the following error messages.
2015-06-09 12:26:54 JST FATAL:  could not locate required checkpoint record
2015-06-09 12:26:54 JST HINT:  If you are not restoring from a backup,
try removing the file ".../backup_label".

5. according to the above hint message, remove $PGDATA/backup_label
and restart a crash recovery

Then you can see that tablespace_map remains in $PGDATA after the recovery ends.

Regards,

-- 
Fujii Masao


-- 
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: Map basebackup tablespaces using a tablespace_map file

[Amit Kapila]

On Tue, Jun 9, 2015 at 12:27 AM, Andrew Dunstan  wrote:

>
> On 06/08/2015 11:16 AM, Amit Kapila wrote:
>>
>>
>> I have to retry that operation, but for me unlink hasn't deleted
>> the file on Windows, may be I am not doing properly, but in
>> anycase why we want to throw error for such a case, why
>> can't we just ignore and create a symlink with the same name.
>>
>
> 1. You realize that in Windows postgres, unlink is actually pgunlink(),
> right? See port.h. If your experiments weren't using that then they weren't
> testing the same thing.
>

Yes, I know that and was using the same version, but the
small problem in my test was that the file name that is used
for unlink was not same as that of actual file in directory, sorry
for the noise.



> 2. If the unlink fails and the file is still there (i.e. pretty much
> everything except the ENOENT case) then creation of the symlink is bound to
> fail anyway.
>
>  I realize our existing code just more or less assumes that that
>> it's a symlink. I think we've probably been a bit careless there.
>>
>>
>> I agree with you that deleting unrelated file with the same name as
>> symlink is not the right thing to do, but not sure throwing error for
>> such a case is better either.
>>
>>
>>
>
> What else would you suggest?


I think Robert and Alvaro also seems to be inclined towards throwing
error for such a case, so let us do that way, but one small point is that
don't you think that similar code in destroy_tablespace_directories() under
label "remove_symlink:" should use similar logic?


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

Re: [HACKERS] pg_stat_archiver issue with aborted archiver

[Michael Paquier]

On Tue, Jun 9, 2015 at 4:23 AM, Fujii Masao  wrote:
> On Mon, Jun 8, 2015 at 5:17 PM, Julien Rouhaud
>  wrote:
>> Le 08/06/2015 05:56, Michael Paquier a écrit :
>>> On Sun, Jun 7, 2015 at 1:11 AM, Julien Rouhaud
>>>  wrote:
 I just noticed that if the archiver aborts (for instance if the
 archive_command exited with a return code > 127),
 pg_stat_archiver won't report those failed attempts. This happens
 with both 9.4 and 9.5 branches.

 Please find attached a patch that fix this issue, based on
 current head.
>>>
>>> The current code seems right to me. When the archive command dies
>>> because of a signal (exit code > 128), the server should fail
>>> immediately with FATAL and should not do any extra processing.
>
> In that case, ISTM that the archiver process dies with FATAL but
> the server not. No? Then the archiver is restarted by postmaster.
> If my understanding is right, it seems worth applying something like
> Julien's patch.

Er, sure. Please understand the archiver process... My point is that
3ad0728 introduced the behavior that we have now in pgarch.c, and that
we should immediately bail out from the archiver process without
interacting with pgstat, the archiver coming back to this file
archiving at restart, and only use pgstat_send_archiver when there is
a status from pgarch_archiveXlog().
-- 
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] Memory leak with XLogFileCopy since de768844 (WAL file with .partial)

[Michael Paquier]

On Tue, Jun 9, 2015 at 6:25 AM, Heikki Linnakangas wrote:
> I'm still not sure if I should've just reverted that refactoring, to make
> XLogFileCopy() look the same in master and back-branches, which makes
> back-patching easier, or keep the refactoring, because it makes the code
> slightly nicer. But the current situation is the worst of both worlds: the
> interface of XLogFileCopy() is no better than it used to be, but it's
> different enough to cause merge conflicts. At this point, it's probably best
> to revert the code to look the same as in 9.4.

That's a valid concern. What about the attached then? I think that it
is still good to keep upto to copy only data up to the switch point at
recovery exit. InstallXLogFileSegment() changes a bit as well because
of its modifications of arguments.
-- 
Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 150d56a..e8d3524 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -807,7 +807,7 @@ static bool XLogCheckpointNeeded(XLogSegNo new_segno);
 static void XLogWrite(XLogwrtRqst WriteRqst, bool flexible);
 static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
 	   bool find_free, XLogSegNo max_segno,
-	   bool use_lock, int elevel);
+	   bool use_lock);
 static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
 			 int source, bool notexistOk);
 static int	XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source);
@@ -3012,7 +3012,7 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
 	max_segno = logsegno + CheckPointSegments;
 	if (!InstallXLogFileSegment(&installed_segno, tmppath,
 *use_existent, max_segno,
-use_lock, LOG))
+use_lock))
 	{
 		/*
 		 * No need for any more future segments, or InstallXLogFileSegment()
@@ -3039,20 +3039,25 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
 }
 
 /*
- * Copy a WAL segment file in pg_xlog directory.
+ * Create a new XLOG file segment by copying a pre-existing one.
  *
- * srcfname		source filename
- * upto			how much of the source file to copy? (the rest is filled with
- *zeros)
- * segno		identify segment to install.
+ * destsegno: identify segment to be created.
  *
- * The file is first copied with a temporary filename, and then installed as
- * a newly-created segment.
+ * srcTLI, srclog, srcseg: identify segment to be copied (could be from
+ *		a different timeline)
+ *
+ * upto: how much of the source file to copy (the rest is filled with
+ *		zeros)
+ *
+ * Currently this is only used during recovery, and so there are no locking
+ * considerations.  But we should be just as tense as XLogFileInit to avoid
+ * emplacing a bogus file.
  */
 static void
-XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
+XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
+			 int upto)
 {
-	char		srcpath[MAXPGPATH];
+	char		path[MAXPGPATH];
 	char		tmppath[MAXPGPATH];
 	char		buffer[XLOG_BLCKSZ];
 	int			srcfd;
@@ -3062,12 +3067,12 @@ XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
 	/*
 	 * Open the source file
 	 */
-	snprintf(srcpath, MAXPGPATH, XLOGDIR "/%s", srcfname);
-	srcfd = OpenTransientFile(srcpath, O_RDONLY | PG_BINARY, 0);
+	XLogFilePath(path, srcTLI, srcsegno);
+	srcfd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0);
 	if (srcfd < 0)
 		ereport(ERROR,
 (errcode_for_file_access(),
- errmsg("could not open file \"%s\": %m", srcpath)));
+ errmsg("could not open file \"%s\": %m", path)));
 
 	/*
 	 * Copy into a temp file name.
@@ -3111,11 +3116,11 @@ XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
 	ereport(ERROR,
 			(errcode_for_file_access(),
 			 errmsg("could not read file \"%s\": %m",
-	srcpath)));
+	path)));
 else
 	ereport(ERROR,
 			(errmsg("not enough data in file \"%s\"",
-	srcpath)));
+	path)));
 			}
 		}
 		errno = 0;
@@ -3148,9 +3153,11 @@ XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
 
 	CloseTransientFile(srcfd);
 
-	/* install the new file */
-	(void) InstallXLogFileSegment(&segno, tmppath, false,
-  0, false, ERROR);
+	/*
+	 * Now move the segment into place with its final name.
+	 */
+	if (!InstallXLogFileSegment(&destsegno, tmppath, false, 0, false))
+		elog(ERROR, "InstallXLogFileSegment should not have failed");
 }
 
 /*
@@ -3177,8 +3184,6 @@ XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
  * place.  This should be TRUE except during bootstrap log creation.  The
  * caller must *not* hold the lock at call.
  *
- * elevel: log level used by this routine.
- *
  * Returns TRUE if the file was installed successfully.  FALSE indicates that
  * max_segno limit was exceeded, or an error occurred while renaming the
  * file into place.
@@ -3186,7 +3191,7 @@ XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
 static bool
 InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,

Re: [HACKERS] [CORE] Restore-reliability mode

[David Gould]

On Mon, 8 Jun 2015 13:03:56 -0300
Claudio Freire  wrote:

> > Ohmygosh, you have to rpm install a bunch of -devel stuff? What a massive
> > hardship.
> 
> It's not about the 5 minutes of compile time, it's about the signalling.
> 
> Just *when* is git ready for testing? You don't know from the outside.
> 
> I do lurk here a lot and still am unsure quite often.
> 
> Even simply releasing an alpha *tarball* would be useful enough. What
> is needed is the signal to test, rather than a fully-built package.

This. The clients I referred to earlier don't even use the rpm packages,
they build from sources. They need to know when it is worthwhile to take a
new set of sources and test. Some sort of labeling about what the contents
are would enable them to do this.

I don't think a monthly snapshot would work as well as the requirement is
knowing that "grouping sets are in" not that "it is July now".

-dg

-- 
David Gouldda...@sonic.net
If simplicity worked, the world would be overrun with insects.


-- 
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] Cancel race condition

[Tom Lane]

Shay Rojansky  writes:
> My questions/comments:
>- Does PostgreSQL *guarantee* that once the connection used to send the
>cancellation request is closed by the server, the cancellation has been
>delivered (as mentioned by Tom)? In other words, should I be designing a
>.NET driver around this behavior?

The signal's been *sent*.  Whether it's been *delivered* is something
you'd have to ask your local kernel hacker.  The POSIX standard appears
to specifically disclaim any such guarantee; in fact, it doesn't even
entirely promise that a self-signal is synchronous.  There are also
issues like what if the target process currently has signals blocked;
does "delivery" mean that the signal handler's been entered, or something
weaker?

In any case, Postgres has always considered that query cancel is a "best
effort" thing, so even if I thought this was 100% portably reliable,
I would not be in favor of promising anything in the docs.

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] [CORE] Restore-reliability mode

[Gavin Flower]

On 09/06/15 00:59, David Gould wrote:

I think Alphas are valuable and useful and even more so if they have release
notes. For example, some of my clients are capable of fetching sources and
building from scratch and filing bug reports and are often interested in
particular new features. They even have staging infrastructure that could
test new postgres releases with real applications. But they don't do it.
They also don't follow -hackers, they don't track git, and they don't have
any easy way to tell if if the new feature they are interested in is
actually complete and ready to test at any particular time. A lot of
features are developed in multiple commits over a period of time and they
see no point in testing until at least most of the feature is complete and
expected to work. But it is not obvious from outside when that happens for
any given feature. For my clients the value of Alpha releases would
mainly be the release notes, or some other mark in the sand that says "As of
Alpha-3 feature X is included and expected to mostly work."

-dg



RELEASE NOTES

I think that having:

1. release notes

2. an Alpha people can simply install without having to compile

Would encourage more people to get involved.  Such people would be 
unlikely to have the time and inclination to use 'nightlies', even if 
compiling was not required.


I have read other posts in this thread, that support the above.

Surely, it would be good for pg to have some more people checking 
quality at an earlier stage?  So reducing barriers to do so is a good thing?



Cheers,
Gavin


--
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] Memory leak with XLogFileCopy since de768844 (WAL file with .partial)

[Heikki Linnakangas]

On 06/08/2015 09:04 PM, Fujii Masao wrote:

On Mon, Jun 8, 2015 at 11:52 AM, Michael Paquier
 wrote:

On Fri, Jun 5, 2015 at 10:45 PM, Fujii Masao wrote:

Why don't we call InstallXLogFileSegment() at the end of XLogFileCopy()?
If we do that, the risk of memory leak you're worried will disappear at all.


Yes, that looks fine, XLogFileCopy() would copy to a temporary file,
then install it definitely. Updated patch attached.


Thanks for updating the patch! Looks good to me. Applied.


XLogFileCopy() used to call InstallXLogFileSegment(), until I refactored 
that in commit de7688442f5aaa03da60416a6aa3474738718803. That commit 
added another caller of XLogFileCopy(), which didn't want to install the 
segment. However, I later partially reverted that patch in commit 
7cbee7c0a1db668c60c020a3fd1e3234daa562a9, and those changes to 
XLogFileCopy() were not really needed anymore. I decided not to revert 
those changes at that point because that refactoring seemed sensible 
anyway. See my email at 
http://www.postgresql.org/message-id/555dd101.7080...@iki.fi about that.


I'm still not sure if I should've just reverted that refactoring, to 
make XLogFileCopy() look the same in master and back-branches, which 
makes back-patching easier, or keep the refactoring, because it makes 
the code slightly nicer. But the current situation is the worst of both 
worlds: the interface of XLogFileCopy() is no better than it used to be, 
but it's different enough to cause merge conflicts. At this point, it's 
probably best to revert the code to look the same as in 9.4.


- Heikki



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

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Alvaro Herrera]

Josh Berkus wrote:
> On 06/08/2015 12:48 PM, Alvaro Herrera wrote:
> > Well, reputation-wise we're already losing every time somebody's server
> > crashes on 9.4.2 and finds it won't start, where it did start fine with
> > 9.4.1.  Maybe they simply wanted to change shared_buffers and the server
> > won't start anymore.  Some people even update binaries with the server
> > running; if they don't restart immediately, it could be several days
> > before it fails to start.  It's pretty scary.
> 
> I'm confused by this discussion.  9.4.3 is released.

The bug was not fixed by 9.4.3.  It was fixed by this commit:

Author: Robert Haas 
Branch: master [068cfadf9] 2015-06-05 09:31:57 -0400
Branch: REL9_4_STABLE [b6a3444fa] 2015-06-05 09:33:52 -0400
Branch: REL9_3_STABLE [2a9b01928] 2015-06-05 09:34:15 -0400

Cope with possible failure of the oldest MultiXact to exist.


-- 
Álvaro Herrerahttp://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: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Josh Berkus]

On 06/08/2015 12:48 PM, Alvaro Herrera wrote:
> Well, reputation-wise we're already losing every time somebody's server
> crashes on 9.4.2 and finds it won't start, where it did start fine with
> 9.4.1.  Maybe they simply wanted to change shared_buffers and the server
> won't start anymore.  Some people even update binaries with the server
> running; if they don't restart immediately, it could be several days
> before it fails to start.  It's pretty scary.

I'm confused by this discussion.  9.4.3 is released.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


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

Re: [HACKERS] pg_stat_*_columns?

[Joel Jacobson]

So I've heard from Magnus Hagander today IRL at our Stockholm PostgreSQL
User Group meeting where we discussed this idea. He told me the overhead in
the statistics collector is mainly when reading from it, not that much when
writing to it.
Magnus idea was to first optimize the collector to make it less of a
problem to collect more data. Sounds like a good thing to do, but maybe
more data in it wouldn't be a problem as long as you don't read too often
from it?

On Mon 8 Jun 2015 at 18:48 Robert Haas  wrote:

> On Fri, Jun 5, 2015 at 7:51 AM, Joel Jacobson  wrote:
> > Would others find it useful to see per column statistics about accesses
> to
> > specific columns?
>
> A probably serious and maybe not entirely obvious problem with this is
> that it would increase the amount of statistical information we keep
> by a pretty large multiple.  How many columns do you have in a typical
> table?  20-30?  That's a lot of data for a statistics collector that,
> regrettably, is already overloaded.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

[HACKERS] Cancel race condition

[Shay Rojansky]

Hi everyone.

I'm working on Npgsql and have run into a race condition when cancelling.
The issue is described in the following 10-year-old thread, and I'd like to
make sure things are still the same:
http://www.postgresql.org/message-id/27126.1126649...@sss.pgh.pa.us

My questions/comments:

   - Does PostgreSQL *guarantee* that once the connection used to send the
   cancellation request is closed by the server, the cancellation has been
   delivered (as mentioned by Tom)? In other words, should I be designing a
   .NET driver around this behavior?
   - If so, may I suggest to update the protocol docs to reflect this (
   http://www.postgresql.org/docs/current/static/protocol-flow.html#AEN103033
   )
   - I'm not sure if there's some sort of feature/request list for protocol
   4, but it may make sense to provide a simpler solution for this problem.
   One example would be for the client to send some sort of numeric ID
   identifying each comment (some autoincrement), and to include that ID when
   cancelling. I'm not sure the benefits are worth the extra payload but it
   may be useful for other functionality as well (tracking/logging)? Just a
   thought.

Thanks,

Shay

Re: [HACKERS] could not truncate directory "pg_subtrans": apparent wraparound

[Dan Langille]

If there's anything I can try on my servers to help diagnose the issues,
please let me know.  If desired, I can arrange access for debugging.

On Sat, Jun 6, 2015 at 12:51 AM, Thomas Munro  wrote:

> On Sat, Jun 6, 2015 at 1:25 PM, Alvaro Herrera 
> wrote:
> > Thomas Munro wrote:
> >
> >> My idea was that if I could get oldestXact == next XID in
> >> TruncateSUBSTRANS, then TransactionIdToPage(oldestXact) for a value of
> >> oldestXact that happens to be immediately after a page boundary (so
> >> that xid % 2048 == 0) might give page number that is >=
> >> latest_page_number, causing SimpleLruTruncate to print that message.
> >> But I can't figure out how to get next XID == oldest XID, because
> >> vacuumdb --freeze --all consumes xids itself, so in my first attempt
> >> at this, next XID is always 3 ahead of the oldest XID when a
> >> checkpoint is run.
> >
> > vacuumdb starts by querying pg_database, which eats one XID.
> >
> > Vacuum itself only uses one XID when vac_truncate_clog() is called.
> > This is called from vac_update_datfrozenxid(), which always happen at
> > the end of each user-invoked VACUUM (so three times for vacuumdb if you
> > have three databases); autovacuum does it also at the end of each run.
> > Maybe you can get autovacuum to quit before doing it.
> >
> > OTOH, if the values in the pg_database entry do not change,
> > vac_truncate_clog is not called, and thus vacuum would finish without
> > consuming an XID.
>
> I have manage to reproduce it a few times but haven't quite found the
> right synchronisation hacks to make it reliable so I'm not posting a
> repro script yet.
>
> I think it's a scary sounding message but very rare and entirely
> harmless (unless you really have wrapped around...).  The fix is
> probably something like: if oldest XID == next XID, then just don't
> call SimpleLruTruncate (truncation is deferred until the next
> checkpoint), or perhaps (if we can confirm this doesn't cause problems
> for dirty pages or that there can't be any dirty pages before cutoff
> page because of the preceding flush (as I suspect)) we could use
> cutoffPage = TransactionIdToPage(oldextXact - 1) if oldest == next, or
> maybe even always.
>
> --
> Thomas Munro
> http://www.enterprisedb.com
>

Re: [HACKERS] reaper should restart archiver even on standby

[Alvaro Herrera]

Fujii Masao wrote:
> Hi,
> 
> When the archiver exits, currently reaper() restarts it only while
> the postmaster state is PM_RUN. This is OK in 9.4 or before because
> the archiver could be running on that state. But in 9.5, we can set
> archive_mode to "always" and start the archiver even on the standby.
> So I think that reaper() should restart the archiver even when
> the postmaster state is PM_RECOVERY or PM_HOT_STANDBY with
> some conditions. Patch attached.

Sounds reasonable, but the patch looks pretty messy.  Can't we create
some common function that would be called both here and on ServerLoop?
We also have sigusr1_handler that starts an archiver -- why does that
one use different conditions?  Also, the block at lines 2807ff seems
like it ought to be changed as well?

-- 
Álvaro Herrerahttp://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: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Joshua D. Drake]

On 06/08/2015 12:48 PM, Alvaro Herrera wrote:


Well, reputation-wise we're already losing every time somebody's server
crashes on 9.4.2 and finds it won't start, where it did start fine with
9.4.1.  Maybe they simply wanted to change shared_buffers and the server
won't start anymore.  Some people even update binaries with the server
running; if they don't restart immediately, it could be several days
before it fails to start.  It's pretty scary.


Yeah no doubt there.

JD


--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


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

[HACKERS] reaper should restart archiver even on standby

[Fujii Masao]

Hi,

When the archiver exits, currently reaper() restarts it only while
the postmaster state is PM_RUN. This is OK in 9.4 or before because
the archiver could be running on that state. But in 9.5, we can set
archive_mode to "always" and start the archiver even on the standby.
So I think that reaper() should restart the archiver even when
the postmaster state is PM_RECOVERY or PM_HOT_STANDBY with
some conditions. Patch attached.

Regards,

-- 
Fujii Masao
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index ee0b018..a4c677a 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2810,7 +2810,10 @@ reaper(SIGNAL_ARGS)
 			if (!EXIT_STATUS_0(exitstatus))
 LogChildExit(LOG, _("archiver process"),
 			 pid, exitstatus);
-			if (XLogArchivingActive() && pmState == PM_RUN)
+			if (wal_level >= WAL_LEVEL_ARCHIVE &&
+((pmState == PM_RUN && XLogArchiveMode > ARCHIVE_MODE_OFF) ||
+ ((pmState == PM_RECOVERY || pmState == PM_HOT_STANDBY) &&
+  XLogArchiveMode == ARCHIVE_MODE_ALWAYS)))
 PgArchPID = pgarch_start();
 			continue;
 		}

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

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Alvaro Herrera]

Joshua D. Drake wrote:
> 
> On 06/08/2015 12:31 PM, Alvaro Herrera wrote:
> >
> >Joshua D. Drake wrote:
> >
> >>If we release on Friday that is the 12th, PgCon is starts the 16th and there
> >>is a weekend in between. If there is an unknown regression or new bug that
> >>is severe, are we going to have the resources to resolve it?
> >
> >ISTM if that happens, we're no worse off than currently.
> 
> Technically sure, reputation?

Well, reputation-wise we're already losing every time somebody's server
crashes on 9.4.2 and finds it won't start, where it did start fine with
9.4.1.  Maybe they simply wanted to change shared_buffers and the server
won't start anymore.  Some people even update binaries with the server
running; if they don't restart immediately, it could be several days
before it fails to start.  It's pretty scary.

> If we really want to do this, let's do it. I am not trying to throw a wrench
> into things. I just really don't want to have to go back, yet again.

Sure, me neither.

-- 
Álvaro Herrerahttp://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: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Joshua D. Drake]

On 06/08/2015 12:31 PM, Alvaro Herrera wrote:


Joshua D. Drake wrote:


If we release on Friday that is the 12th, PgCon is starts the 16th and there
is a weekend in between. If there is an unknown regression or new bug that
is severe, are we going to have the resources to resolve it?


ISTM if that happens, we're no worse off than currently.


Technically sure, reputation?

If we really want to do this, let's do it. I am not trying to throw a 
wrench into things. I just really don't want to have to go back, yet again.


Sincerely,

JD



--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


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

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Alvaro Herrera]

Joshua D. Drake wrote:

> If we release on Friday that is the 12th, PgCon is starts the 16th and there
> is a weekend in between. If there is an unknown regression or new bug that
> is severe, are we going to have the resources to resolve it?

ISTM if that happens, we're no worse off than currently.

-- 
Álvaro Herrerahttp://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: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Joshua D. Drake]

On 06/08/2015 12:21 PM, Tom Lane wrote:

Andres Freund  writes:

On 2015-06-08 14:18:22 -0400, Tom Lane wrote:

As I saw it, on Friday it was not clear whether we would be able to do a
release this week.  Now it's Monday, and we still have a rather long list
of issues



Well, these issues aren't regressions, they're "just" general problems
we need to fix. And some of them are going to require somewhat invasive
changes. Both you and Robert have argued that the regressions should be
fixed first. And by now you've convinced me.



, and apparently Andres isn't all that happy even with the fixes
that have gone in, because he still wants more time for testing.



I'm now satisfied that the current HEAD is better than what was released
last time round.


If there's general agreement that there are no regressions from
9.4.anything, then perhaps we should put out a release this week.
Wrap today seems out of the question but we could still do it tomorrow
for Friday release.


If we release on Friday that is the 12th, PgCon is starts the 16th and 
there is a weekend in between. If there is an unknown regression or new 
bug that is severe, are we going to have the resources to resolve it?


Sincerely,

JD

--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


--
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] last_analyze/last_vacuum not being updated

[Tom Lane]

Peter Eisentraut  writes:
> I'm looking at a case on 9.4.1 where the last_analyze and last_vacuum
> stats for a handful of tables seem stuck.  They don't update after
> running an ANALYZE or VACUUM command, and they don't react to
> pg_stat_reset_single_table_counters().  All of the affected tables are
> system catalogs, some shared, some not.  Other system catalogs and other
> tables have their statistics updated normally.  Any ideas (before I try
> to blow it away)?

Hmm ... corrupted hash table inside the stats collector, perhaps?
It would be interesting to look at the stats disk file and see if
there are multiple entries for those OIDs, or if VACUUMing one of
them causes some *other* table's last_vacuum to advance.

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] last_analyze/last_vacuum not being updated

[Alvaro Herrera]

Peter Eisentraut wrote:
> I'm looking at a case on 9.4.1 where the last_analyze and last_vacuum
> stats for a handful of tables seem stuck.  They don't update after
> running an ANALYZE or VACUUM command, and they don't react to
> pg_stat_reset_single_table_counters().  All of the affected tables are
> system catalogs, some shared, some not.  Other system catalogs and other
> tables have their statistics updated normally.  Any ideas (before I try
> to blow it away)?

Interesting.  Not sure what would cause this.  Maybe their pgstat_info
pointer is invalid in their relcache entry for some reason.  Are these
rd_isnailed?

-- 
Álvaro Herrerahttp://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] pg_stat_archiver issue with aborted archiver

[Fujii Masao]

On Mon, Jun 8, 2015 at 5:17 PM, Julien Rouhaud
 wrote:
> Le 08/06/2015 05:56, Michael Paquier a écrit :
>> On Sun, Jun 7, 2015 at 1:11 AM, Julien Rouhaud
>>  wrote:
>>> I just noticed that if the archiver aborts (for instance if the
>>> archive_command exited with a return code > 127),
>>> pg_stat_archiver won't report those failed attempts. This happens
>>> with both 9.4 and 9.5 branches.
>>>
>>> Please find attached a patch that fix this issue, based on
>>> current head.
>>
>> The current code seems right to me. When the archive command dies
>> because of a signal (exit code > 128), the server should fail
>> immediately with FATAL and should not do any extra processing.

In that case, ISTM that the archiver process dies with FATAL but
the server not. No? Then the archiver is restarted by postmaster.
If my understanding is right, it seems worth applying something like
Julien's patch.

Regards,

-- 
Fujii Masao


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

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Tom Lane]

Andres Freund  writes:
> On 2015-06-08 14:18:22 -0400, Tom Lane wrote:
>> As I saw it, on Friday it was not clear whether we would be able to do a
>> release this week.  Now it's Monday, and we still have a rather long list
>> of issues

> Well, these issues aren't regressions, they're "just" general problems
> we need to fix. And some of them are going to require somewhat invasive
> changes. Both you and Robert have argued that the regressions should be
> fixed first. And by now you've convinced me.

>> , and apparently Andres isn't all that happy even with the fixes
>> that have gone in, because he still wants more time for testing.

> I'm now satisfied that the current HEAD is better than what was released
> last time round.

If there's general agreement that there are no regressions from
9.4.anything, then perhaps we should put out a release this week.
Wrap today seems out of the question but we could still do it tomorrow
for Friday release.

Given the lack of notice, I doubt that all the packagers would be on board
promptly; but as long as it's not a security release there's no urgent
reason that they all have to be ready at the same 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

[HACKERS] last_analyze/last_vacuum not being updated

[Peter Eisentraut]

I'm looking at a case on 9.4.1 where the last_analyze and last_vacuum
stats for a handful of tables seem stuck.  They don't update after
running an ANALYZE or VACUUM command, and they don't react to
pg_stat_reset_single_table_counters().  All of the affected tables are
system catalogs, some shared, some not.  Other system catalogs and other
tables have their statistics updated normally.  Any ideas (before I try
to blow it away)?


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

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Robert Haas]

On Mon, Jun 8, 2015 at 2:18 PM, Tom Lane  wrote:
> As I saw it, on Friday it was not clear whether we would be able to do a
> release this week.  Now it's Monday, and we still have a rather long list
> of issues, and apparently Andres isn't all that happy even with the fixes
> that have gone in, because he still wants more time for testing.

My reading is that Andres is convinced that we've fixed the
regressions in 9.4.2.

-- 
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] Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file

[Andrew Dunstan]

On 06/08/2015 11:16 AM, Amit Kapila wrote:
On Mon, Jun 8, 2015 at 6:39 PM, Andrew Dunstan > wrote:



On 06/08/2015 12:08 AM, Amit Kapila wrote:

How about if it is just a flat file with same name as
tablespace link,
why we want to give error for that case?  I think now it just
don't do
anything with that file (unlink will fail with ENOENT and it
will be
ignored, atleast thats the way currently it behaves in
Windows) and
create a separate symlink with same name which seems okay to
me and in the change proposed by you it will give error, do
you see
any reason for doing so?




This is surely wrong. unlink won't fail with ENOENT if the file is
present; ENOENT means that the file is NOT present. It will
succeed if the file is present, which is exactly what I'm saying
is wrong.


I have to retry that operation, but for me unlink hasn't deleted
the file on Windows, may be I am not doing properly, but in
anycase why we want to throw error for such a case, why
can't we just ignore and create a symlink with the same name.


1. You realize that in Windows postgres, unlink is actually pgunlink(), 
right? See port.h. If your experiments weren't using that then they 
weren't testing the same thing.


2. If the unlink fails and the file is still there (i.e. pretty much 
everything except the ENOENT case) then creation of the symlink is bound 
to fail anyway.



I realize our existing code just more or less assumes that that
it's a symlink. I think we've probably been a bit careless there.


I agree with you that deleting unrelated file with the same name as
symlink is not the right thing to do, but not sure throwing error for
such a case is better either.





What else would you suggest? Closing our eyes and wishing it weren't so 
doesn't seem like a solution.


cheers

andrew


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

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Andres Freund]

On 2015-06-08 14:18:22 -0400, Tom Lane wrote:
> As I saw it, on Friday it was not clear whether we would be able to do a
> release this week.  Now it's Monday, and we still have a rather long list
> of issues

Well, these issues aren't regressions, they're "just" general problems
we need to fix. And some of them are going to require somewhat invasive
changes. Both you and Robert have argued that the regressions should be
fixed first. And by now you've convinced me.

>, and apparently Andres isn't all that happy even with the fixes
> that have gone in, because he still wants more time for testing.

I'm now satisfied that the current HEAD is better than what was released
last time round.

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: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Tom Lane]

Robert Haas  writes:
> It's not exactly going into a black hole, but there was some
> communication between Tom and Andres on Friday that left Andres with
> the impression that if he spent the weekend testing the new code for
> problems and things went well, we'd be able to get a release this
> week.  So he spent his weekend on that, rather than, saying, doing
> something fun, and now Tom wants to wait two weeks.  I'm not accusing
> anybody of anything, but if Andres felt like beating his head against
> a nearby wall at this point, I'd sympathize.

As I saw it, on Friday it was not clear whether we would be able to do a
release this week.  Now it's Monday, and we still have a rather long list
of issues, and apparently Andres isn't all that happy even with the fixes
that have gone in, because he still wants more time for testing.

Are we really benefiting anyone if we force out a rushed release right
now?  What are the odds that it will make things worse?

> Obviously, we need to do what is best for the project overall, not
> what is best for any individual developer's cranial integrity.  But
> the decision-making process here is not entirely clear, and it's not
> entirely obvious that we're making the right ones.

AFAICT, you've been in on every single email thread discussing schedule
over the past couple of weeks, and so has Andres.  If you think it's
unclear, it's not because somebody is hiding something from you, it's
because it *is* unclear what we ought to do.

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: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Andres Freund]

On 2015-06-08 13:16:00 -0400, Robert Haas wrote:
> It's not exactly going into a black hole, but there was some
> communication between Tom and Andres on Friday that left Andres with
> the impression that if he spent the weekend testing the new code for
> problems and things went well, we'd be able to get a release this
> week.

More precisely I felt rather unsure whether we'd release on Monday,
Tuesday, or not at all. And I'd rather have a tested release out there
than an untested one.

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] Memory leak with XLogFileCopy since de768844 (WAL file with .partial)

[Fujii Masao]

On Mon, Jun 8, 2015 at 11:52 AM, Michael Paquier
 wrote:
> On Fri, Jun 5, 2015 at 10:45 PM, Fujii Masao wrote:
>> Why don't we call InstallXLogFileSegment() at the end of XLogFileCopy()?
>> If we do that, the risk of memory leak you're worried will disappear at all.
>
> Yes, that looks fine, XLogFileCopy() would copy to a temporary file,
> then install it definitely. Updated patch attached.

Thanks for updating the patch! Looks good to me. Applied.

Regards,

-- 
Fujii Masao


-- 
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] Dependency between bgw_notify_pid and bgw_flags

[Robert Haas]

On Mon, Jun 8, 2015 at 1:51 PM, Alvaro Herrera  wrote:
> Robert Haas wrote:
>
>> After studying this, I think it's a bug.  See this comment:
>>
>>  * Normal child backends can only be launched when we are in PM_RUN or
>>  * PM_HOT_STANDBY state.  (We also allow launch of normal
>>  * child backends in PM_WAIT_BACKUP state, but only for superusers.)
>>  * In other states we handle connection requests by launching "dead_end"
>>  * child processes, which will simply send the client an error message and
>>  * quit.  (We track these in the BackendList so that we can know when they
>>  * are all gone; this is important because they're still connected to shared
>>  * memory, and would interfere with an attempt to destroy the shmem segment,
>>  * possibly leading to SHMALL failure when we try to make a new one.)
>>  * In PM_WAIT_DEAD_END state we are waiting for all the dead_end children
>>  * to drain out of the system, and therefore stop accepting connection
>>  * requests at all until the last existing child has quit (which hopefully
>>  * will not be very long).
>>
>> That comment seems to imply that, at the very least, all backends with
>> shared-memory access need to be part of BackendList.  But really, I'm
>> unclear why we'd ever want to exit without all background workers
>> having shut down, so maybe they should all be in there.  Isn't that
>> sorta the point of this facility?
>>
>> Alvaro, any thoughts?
>
> As I recall, my thinking was:
>
> * anything connected to shmem that crashes could leave things in bad
> state (for example locks improperly held), whereas things not connected
> to shmem could crash without it being a problem for the rest of the
> system and thus do not require a full restart cycle.  This stuff is
> detected with the PMChildSlot thingy; therefore all bgworkers with shmem
> access should have one of these.
>
> * I don't recall offhand what the criteria is for stuff to be in
> postmaster's BackendList.  According to the comment on top of struct
> Backend, bgworkers connected to shmem should be on that list, even if
> they did not have the BGWORKER_BACKEND_DATABASE_CONNECTION flag on
> registration.  So that would be a bug.
>
> Does this help you any?

Well, it sounds like we at least need to think about making it
consistently depend on shmem-access rather than database-connection.
I'm less sure what to do with workers that have not even got shmem
access.

-- 
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] Restore-reliability mode

[Bruce Momjian]

On Mon, Jun  8, 2015 at 07:48:36PM +0200, Andres Freund wrote:
> On 2015-06-08 13:44:05 -0400, Bruce Momjian wrote:
> > I understand the overreaction/underreaction debate.  Here were my goals
> > in this discussion:
> > 
> > 1.  stop worry about the 9.5 timeline so we could honestly assess our
> > software - *done*
> > 2.  seriously address multi-xact issues without 9.5/commit-fest pressure -
> > *in process*
> > 3.  identify any other areas in need of serious work
> > 
> > While I like the list you provided, I don't think we can be effective in
> > an environment where we assume every big new features will have problems
> > like multi-xact.  For example, we have not seen destabilization from any
> > major 9.4 features, that I can remember anyway.
> > 
> > Unless there is consensus about new areas for #3, I am thinking we will
> > continue looking at multi-xact until we are happy, then move ahead with
> > 9.5 items in the way we have before.
> 
> I think one important part is that we (continue to?) regularly tell our
> employers that work on pre-commit, post-commit review, and refactoring
> are critical for their long term business prospects.  My impression so
> far is that that the employer side hasn't widely realized that fact, and
> that many contributors do the review etc. part in their spare time.

Agreed.  My bet is that more employers realize it now than they did a
few months ago --- kind of hard to miss all those minor releases and
customer complaints.  :-(

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
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] Dependency between bgw_notify_pid and bgw_flags

[Alvaro Herrera]

Robert Haas wrote:

> After studying this, I think it's a bug.  See this comment:
> 
>  * Normal child backends can only be launched when we are in PM_RUN or
>  * PM_HOT_STANDBY state.  (We also allow launch of normal
>  * child backends in PM_WAIT_BACKUP state, but only for superusers.)
>  * In other states we handle connection requests by launching "dead_end"
>  * child processes, which will simply send the client an error message and
>  * quit.  (We track these in the BackendList so that we can know when they
>  * are all gone; this is important because they're still connected to shared
>  * memory, and would interfere with an attempt to destroy the shmem segment,
>  * possibly leading to SHMALL failure when we try to make a new one.)
>  * In PM_WAIT_DEAD_END state we are waiting for all the dead_end children
>  * to drain out of the system, and therefore stop accepting connection
>  * requests at all until the last existing child has quit (which hopefully
>  * will not be very long).
> 
> That comment seems to imply that, at the very least, all backends with
> shared-memory access need to be part of BackendList.  But really, I'm
> unclear why we'd ever want to exit without all background workers
> having shut down, so maybe they should all be in there.  Isn't that
> sorta the point of this facility?
> 
> Alvaro, any thoughts?

As I recall, my thinking was:

* anything connected to shmem that crashes could leave things in bad
state (for example locks improperly held), whereas things not connected
to shmem could crash without it being a problem for the rest of the
system and thus do not require a full restart cycle.  This stuff is
detected with the PMChildSlot thingy; therefore all bgworkers with shmem
access should have one of these.

* I don't recall offhand what the criteria is for stuff to be in
postmaster's BackendList.  According to the comment on top of struct
Backend, bgworkers connected to shmem should be on that list, even if
they did not have the BGWORKER_BACKEND_DATABASE_CONNECTION flag on
registration.  So that would be a bug.

Does this help you any?

-- 
Álvaro Herrerahttp://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] Restore-reliability mode

[Andres Freund]

On 2015-06-08 13:44:05 -0400, Bruce Momjian wrote:
> I understand the overreaction/underreaction debate.  Here were my goals
> in this discussion:
> 
> 1.  stop worry about the 9.5 timeline so we could honestly assess our
> software - *done*
> 2.  seriously address multi-xact issues without 9.5/commit-fest pressure -
> *in process*
> 3.  identify any other areas in need of serious work
> 
> While I like the list you provided, I don't think we can be effective in
> an environment where we assume every big new features will have problems
> like multi-xact.  For example, we have not seen destabilization from any
> major 9.4 features, that I can remember anyway.
> 
> Unless there is consensus about new areas for #3, I am thinking we will
> continue looking at multi-xact until we are happy, then move ahead with
> 9.5 items in the way we have before.

I think one important part is that we (continue to?) regularly tell our
employers that work on pre-commit, post-commit review, and refactoring
are critical for their long term business prospects.  My impression so
far is that that the employer side hasn't widely realized that fact, and
that many contributors do the review etc. part in their spare time.

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] Restore-reliability mode

[Bruce Momjian]

On Sat, Jun  6, 2015 at 03:58:05PM -0400, Noah Misch wrote:
> On Fri, Jun 05, 2015 at 08:25:34AM +0100, Simon Riggs wrote:
> > This whole idea of "feature development" vs reliability is bogus. It
> > implies people that work on features don't care about reliability. Given
> > the fact that many of the features are actually about increasing database
> > reliability in the event of crashes and corruptions it just makes no sense.
> 
> I'm contrasting work that helps to keep our existing promises ("reliability")
> with work that makes new promises ("features").  In software development, we
> invariably hazard old promises to make new promises; our success hinges on
> electing neither too little nor too much risk.  Two years ago, PostgreSQL's
> track record had placed it in a good position to invest in new, high-risk,
> high-reward promises.  We did that, and we emerged solvent yet carrying an
> elevated debt service ratio.  It's time to reduce risk somewhat.
> 
> You write about a different sense of "reliability."  (Had I anticipated this
> misunderstanding, I might have written "Restore-probity mode.")  None of this
> was about classifying people, most of whom allocate substantial time to each
> kind of work.
> 
> > How will we participate in cleanup efforts? How do we know when something
> > has been "cleaned up", how will we measure our success or failure? I think
> > we should be clear that wasting N months on cleanup can *fail* to achieve a
> > useful objective. Without a clear plan it almost certainly will do so. The
> > flip side is that wasting N months will cause great amusement and dancing
> > amongst those people who wish to pull ahead of our open source project and
> > we should take care not to hand them a victory from an overreaction.
> 
> I agree with all that.  We should likewise take care not to become insolvent
> from an underreaction.

I understand the overreaction/underreaction debate.  Here were my goals
in this discussion:

1.  stop worry about the 9.5 timeline so we could honestly assess our
software - *done*
2.  seriously address multi-xact issues without 9.5/commit-fest pressure -
*in process*
3.  identify any other areas in need of serious work

While I like the list you provided, I don't think we can be effective in
an environment where we assume every big new features will have problems
like multi-xact.  For example, we have not seen destabilization from any
major 9.4 features, that I can remember anyway.

Unless there is consensus about new areas for #3, I am thinking we will
continue looking at multi-xact until we are happy, then move ahead with
9.5 items in the way we have before.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
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: Map basebackup tablespaces using a tablespace_map file

[Alvaro Herrera]

Robert Haas wrote:

> Why not?  I think that if we encounter some sort of situation that we
> think should never happen, throwing an error is exactly what we
> *should* do.  Particularly when it comes to things like removing
> files, it is very dangerous for the database to proceed if the
> situation is not as expected.  We should only remove things if we are
> quite sure that removing them is the right thing to do.

Yes, agreed.

I notice that we use %m in places where I'm not sure errno is the right
thing.  Consider the ereport() at lines 10385ff, for instance.  I don't
think fgetc() nor ferror() set errno.

I became aware of this because last week I was reading some bogus
pg_dump code that reported "could not write foobar: Success" and noticed
that the macros READ_ERROR_EXIT and WRITE_ERROR_EXIT also do
strerror(errno) after doing some fread() or similar.

-- 
Álvaro Herrerahttp://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] [GENERAL] 9.4.1 -> 9.4.2 problem: could not access status of transaction 1

[Alvaro Herrera]

Robert Haas wrote:
> On Mon, Jun 8, 2015 at 1:23 PM, Alvaro Herrera  
> wrote:

> > (My personal alarm bells go off when I see autovac_naptime=15min or
> > more, but apparently not everybody sees things that way.)
> 
> Uh, I'd echo that sentiment if you did s/15min/1min/

Yeah, well, that too I guess.

> I think Andres's patch is just improving the existing mechanism so
> that it's reliable, and you're proposing something notably different
> which might be better, but which is really a different proposal
> altogether.

Fair enough.

-- 
Álvaro Herrerahttp://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] [GENERAL] 9.4.1 -> 9.4.2 problem: could not access status of transaction 1

[Robert Haas]

On Mon, Jun 8, 2015 at 1:23 PM, Alvaro Herrera  wrote:
> Andres Freund wrote:
>> On June 8, 2015 7:06:31 PM GMT+02:00, Alvaro Herrera 
>>  wrote:
>> >I might be misreading the code, but PMSIGNAL_START_AUTOVAC_LAUNCHER
>> >only causes things to happen (i.e. a new worker to be started) when
>> >autovacuum is disabled.  If autovacuum is enabled, postmaster
>> >receives the signal and doesn't do anything about it, because the
>> >launcher is already running.  Of course, regularly scheduled autovac
>> >workers will eventually start running, but perhaps this is not good
>> >enough.
>>
>> Well that's just the same for the plain xid precedent? I'd not mind
>> improving further, but that seems like a separate thing.
>
> Sure.  I just concern that we might be putting excessive trust on
> emergency workers being launched at a high pace.  With normally
> configured systems (naptime=1min) it shouldn't be a problem, but we have
> seen systems with naptime set to one hour or so, and those might feel
> some pain; and it would get worse the more databases you have, because
> people might feel the need to space the autovac runs even more.
>
> (My personal alarm bells go off when I see autovac_naptime=15min or
> more, but apparently not everybody sees things that way.)

Uh, I'd echo that sentiment if you did s/15min/1min/

I think Andres's patch is just improving the existing mechanism so
that it's reliable, and you're proposing something notably different
which might be better, but which is really a different proposal
altogether.

-- 
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] [GENERAL] 9.4.1 -> 9.4.2 problem: could not access status of transaction 1

[Andres Freund]

On 2015-06-08 14:23:32 -0300, Alvaro Herrera wrote:
> Sure.  I just concern that we might be putting excessive trust on
> emergency workers being launched at a high pace.

I'm not sure what to do about that. I mean, it'd not be hard to simply
ignore naptime upon wraparound, but I'm not sure that'd be well
received.

> (My personal alarm bells go off when I see autovac_naptime=15min or
> more, but apparently not everybody sees things that way.)

Understandably so. I'd be alarmed at much lower values than that
actually.

> > --- 
> > Please excuse brevity and formatting - I am writing this on my mobile phone.
> 
> I wonder if these notices are useful at all.

I only know that I'm less annoyed at reading a untrimmed/badly wrapped
email if it's sent from a mobile phone, where it's hard to impossible to
write a well formatted email, than when sent from a full desktop.
That's why I added the notice...

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] Dependency between bgw_notify_pid and bgw_flags

[Robert Haas]

On Thu, Jun 4, 2015 at 8:52 AM, Ashutosh Bapat
 wrote:
> Documentation here http://www.postgresql.org/docs/devel/static/bgworker.html
> does not indicate any relation between the fields bgw_notify_pid and
> bgw_flags of BackgroundWorker structure. But in one has to set
> BGWORKER_BACKEND_DATABASE_CONNECTION in order to use bgw_notify_pid feature.

The comments in maybe_start_bgworker make it fairly clear that this
was done intentionally, but they don't explain why:

 * If not connected, we don't need a Backend
element, but we still
 * need a PMChildSlot.

But there's another comment elsewhere that says that the criteria is
shared-memory access, not database connection:

 * Background workers that request shared memory access during registration are
 * in this list, too.

So at a minimum, the current situation is not self-consistent.

After studying this, I think it's a bug.  See this comment:

 * Normal child backends can only be launched when we are in PM_RUN or
 * PM_HOT_STANDBY state.  (We also allow launch of normal
 * child backends in PM_WAIT_BACKUP state, but only for superusers.)
 * In other states we handle connection requests by launching "dead_end"
 * child processes, which will simply send the client an error message and
 * quit.  (We track these in the BackendList so that we can know when they
 * are all gone; this is important because they're still connected to shared
 * memory, and would interfere with an attempt to destroy the shmem segment,
 * possibly leading to SHMALL failure when we try to make a new one.)
 * In PM_WAIT_DEAD_END state we are waiting for all the dead_end children
 * to drain out of the system, and therefore stop accepting connection
 * requests at all until the last existing child has quit (which hopefully
 * will not be very long).

That comment seems to imply that, at the very least, all backends with
shared-memory access need to be part of BackendList.  But really, I'm
unclear why we'd ever want to exit without all background workers
having shut down, so maybe they should all be in there.  Isn't that
sorta the point of this facility?

Alvaro, any thoughts?

-- 
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] [GENERAL] 9.4.1 -> 9.4.2 problem: could not access status of transaction 1

[Alvaro Herrera]

Andres Freund wrote:
> On June 8, 2015 7:06:31 PM GMT+02:00, Alvaro Herrera 
>  wrote:

> >I might be misreading the code, but PMSIGNAL_START_AUTOVAC_LAUNCHER
> >only causes things to happen (i.e. a new worker to be started) when
> >autovacuum is disabled.  If autovacuum is enabled, postmaster
> >receives the signal and doesn't do anything about it, because the
> >launcher is already running.  Of course, regularly scheduled autovac
> >workers will eventually start running, but perhaps this is not good
> >enough.
> 
> Well that's just the same for the plain xid precedent? I'd not mind
> improving further, but that seems like a separate thing.

Sure.  I just concern that we might be putting excessive trust on
emergency workers being launched at a high pace.  With normally
configured systems (naptime=1min) it shouldn't be a problem, but we have
seen systems with naptime set to one hour or so, and those might feel
some pain; and it would get worse the more databases you have, because
people might feel the need to space the autovac runs even more.

(My personal alarm bells go off when I see autovac_naptime=15min or
more, but apparently not everybody sees things that way.)

> --- 
> Please excuse brevity and formatting - I am writing this on my mobile phone.

I wonder if these notices are useful at all.

-- 
Álvaro Herrerahttp://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] [idea] more aggressive join pushdown on postgres_fdw

[Robert Haas]

On Fri, Jun 5, 2015 at 5:51 AM, Shigeru HANADA  wrote:
> 2015/06/05 6:43、Robert Haas  のメール：
>> On Sat, May 30, 2015 at 9:03 PM, Kouhei Kaigai  wrote:
>> Neat idea.  This ties into something I've thought about and mentioned
>> before: what if the innerrel is local, but there's a replicated copy
>> on the remote server?  Perhaps both cases are worth thinking about at
>> some point.
>
> Interesting, but I’m not sure that I understood the situation.
>
> Here which kind of replication method do you mean?  I guess you assume some 
> kind of per-table replication such as Slony-I or materialized views with 
> postgres_fdw or dblink, in postgres_fdw case.  If this assumption is correct, 
> we need a mapping between a local ordinary table and a foreign table which 
> points remote replicated table.

Right.  I was thinking of BDR, in particular, or some future future
in-core feature which might be similar, but Slony could do the same
thing.

-- 
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: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Alvaro Herrera]

Bruce Momjian wrote:
> On Mon, Jun  8, 2015 at 02:01:52PM -0300, Alvaro Herrera wrote:

> > > OK, are these fixed in 9.4.2 or would the same failure happen in 9.4.3?
> > 
> > The fixes are not yet in any released branch, hence the rush to get
> > these out.
> 
> OK, now I understand.  :-O  We have known failures that are not patched,
> hence the desire for a release.  

Part of the problem is that they are regressions: these systems did not
have any trouble with 9.4.1/9.3.6 (other than being at risk of members
overrun, of course.)

> I am a little concerned we are getting into a case where community
> members dedicated to this issue are asking for a release, and it is
> going into the core black hole, meaning there is no visibility on what
> actions core is taking to make a decision.

At 2ndQuadrant, and I imagine EDB is in the same position, we have
enough packaging stuff going on that we can ship patched releases to
customers in case of trouble.  I worry about users not having that
privilege.

-- 
Álvaro Herrerahttp://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: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Robert Haas]

On Mon, Jun 8, 2015 at 1:08 PM, Bruce Momjian  wrote:
> OK, now I understand.  :-O  We have known failures that are not patched,
> hence the desire for a release.
>
> I am a little concerned we are getting into a case where community
> members dedicated to this issue are asking for a release, and it is
> going into the core black hole, meaning there is no visibility on what
> actions core is taking to make a decision.  (This is not a criticism,
> but rather an observation of how it looks from a non-core-member
> perspective.)

It's not exactly going into a black hole, but there was some
communication between Tom and Andres on Friday that left Andres with
the impression that if he spent the weekend testing the new code for
problems and things went well, we'd be able to get a release this
week.  So he spent his weekend on that, rather than, saying, doing
something fun, and now Tom wants to wait two weeks.  I'm not accusing
anybody of anything, but if Andres felt like beating his head against
a nearby wall at this point, I'd sympathize.

Obviously, we need to do what is best for the project overall, not
what is best for any individual developer's cranial integrity.  But
the decision-making process here is not entirely clear, and it's not
entirely obvious that we're making the right ones.

-- 
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] [GENERAL] 9.4.1 -> 9.4.2 problem: could not access status of transaction 1

[Andres Freund]

On June 8, 2015 7:06:31 PM GMT+02:00, Alvaro Herrera  
wrote:
>Andres Freund wrote:
>
>> A first version to address this problem can be found appended to this
>> email.
>> 
>> Basically it does:
>> * Whenever more than MULTIXACT_MEMBER_SAFE_THRESHOLD are used, signal
>>   autovacuum once per members segment
>> * For both members and offsets, once hitting the hard limits, signal
>>   autovacuum everytime. Otherwise we loose the information when
>>   restarting the database, or when autovac is killed. I ran into this
>a
>>   bunch of times while testing.
>
>I might be misreading the code, but PMSIGNAL_START_AUTOVAC_LAUNCHER
>only
>causes things to happen (i.e. a new worker to be started) when
>autovacuum is disabled.  If autovacuum is enabled, postmaster receives
>the signal and doesn't do anything about it, because the launcher is
>already running.  Of course, regularly scheduled autovac workers will
>eventually start running, but perhaps this is not good enough.

Well that's just the same for the plain xid precedent? I'd not mind improving 
further, but that seems like a separate thing.

Andres

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.


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

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Bruce Momjian]

On Mon, Jun  8, 2015 at 02:01:52PM -0300, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > On Mon, Jun  8, 2015 at 12:39:24PM -0400, Robert Haas wrote:
> > > On Mon, Jun 8, 2015 at 12:36 PM, Bruce Momjian  wrote:
> > > > Yeah, I think if we needed this out in an emergency, we would do it, but
> > > > based on the volume of recent releases, it would be hard.  Are we seeing
> > > > user reports of failures even on the newest released versions, or are
> > > > these preventive fixes?
> > > 
> > > User reports of failures.  See the thread about upgrading from 9.4.1
> > > to 9.4.2 and having the server *fail to start*.
> > 
> > OK, are these fixed in 9.4.2 or would the same failure happen in 9.4.3?
> 
> The fixes are not yet in any released branch, hence the rush to get
> these out.

OK, now I understand.  :-O  We have known failures that are not patched,
hence the desire for a release.  

I am a little concerned we are getting into a case where community
members dedicated to this issue are asking for a release, and it is
going into the core black hole, meaning there is no visibility on what
actions core is taking to make a decision.  (This is not a criticism,
but rather an observation of how it looks from a non-core-member
perspective.)

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
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] [CORE] Restore-reliability mode

[Magnus Hagander]

On Mon, Jun 8, 2015 at 7:01 PM, Alvaro Herrera 
wrote:

> David G. Johnston wrote:
> > On Mon, Jun 8, 2015 at 12:14 PM, Geoff Winkless 
> wrote:
>
> > > ​I can see that, and can absolutely get behind the idea of a nightly
> being
> > > flagged as an alpha, since it should involve next to no developer time.
> > >
> > ​Nightly where?  This is an international community.
>
> A "nightly" refers to our development snapshots, which are uploaded to
> the ftp servers every "night" (according to some timezone).  You can
> find them in pub/snapshot/ for each branch.
>

Snapshots are actually not nightly anymore, and haven't been for some time.
They are currently run every 4 hours, and are uploaded to the ftp server
once a buildfarm run (on debian x64) finishes.

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

Re: [HACKERS] [GENERAL] 9.4.1 -> 9.4.2 problem: could not access status of transaction 1

[Alvaro Herrera]

Andres Freund wrote:

> A first version to address this problem can be found appended to this
> email.
> 
> Basically it does:
> * Whenever more than MULTIXACT_MEMBER_SAFE_THRESHOLD are used, signal
>   autovacuum once per members segment
> * For both members and offsets, once hitting the hard limits, signal
>   autovacuum everytime. Otherwise we loose the information when
>   restarting the database, or when autovac is killed. I ran into this a
>   bunch of times while testing.

I might be misreading the code, but PMSIGNAL_START_AUTOVAC_LAUNCHER only
causes things to happen (i.e. a new worker to be started) when
autovacuum is disabled.  If autovacuum is enabled, postmaster receives
the signal and doesn't do anything about it, because the launcher is
already running.  Of course, regularly scheduled autovac workers will
eventually start running, but perhaps this is not good enough.

-- 
Álvaro Herrerahttp://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] [CORE] Restore-reliability mode

[Bruce Momjian]

On Mon, Jun  8, 2015 at 12:32:45PM -0400, David G. Johnston wrote:
> On Mon, Jun 8, 2015 at 12:14 PM, Geoff Winkless  wrote:
> 
> On 8 June 2015 at 17:03, Claudio Freire  wrote:
> 
> It's not about the 5 minutes of compile time, it's about the
> signalling.
> 
> Just *when* is git ready for testing? You don't know from the outside.
> 
> I do lurk here a lot and still am unsure quite often.
> 
> Even simply releasing an alpha *tarball* would be useful enough. What
> is needed is the signal to test, rather than a fully-built package.
> 
> 
> ​I can see that, and can absolutely get behind the idea of a nightly being
> flagged as an alpha, since it should involve next to no developer time.
> 
> 
> 
> ​Nightly where?  This is an international community.

The daily snapshot tarballs are built in a way to minimize the number of
development tools required:

http://www.postgresql.org/ftp/snapshot/dev/

These would be easier to use than pulling from git.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
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] [CORE] Restore-reliability mode

[Alvaro Herrera]

David G. Johnston wrote:
> On Mon, Jun 8, 2015 at 12:14 PM, Geoff Winkless  wrote:

> > ​I can see that, and can absolutely get behind the idea of a nightly being
> > flagged as an alpha, since it should involve next to no developer time.
> >
> ​Nightly where?  This is an international community.

A "nightly" refers to our development snapshots, which are uploaded to
the ftp servers every "night" (according to some timezone).  You can
find them in pub/snapshot/ for each branch.

-- 
Álvaro Herrerahttp://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: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Alvaro Herrera]

Bruce Momjian wrote:
> On Mon, Jun  8, 2015 at 12:39:24PM -0400, Robert Haas wrote:
> > On Mon, Jun 8, 2015 at 12:36 PM, Bruce Momjian  wrote:
> > > Yeah, I think if we needed this out in an emergency, we would do it, but
> > > based on the volume of recent releases, it would be hard.  Are we seeing
> > > user reports of failures even on the newest released versions, or are
> > > these preventive fixes?
> > 
> > User reports of failures.  See the thread about upgrading from 9.4.1
> > to 9.4.2 and having the server *fail to start*.
> 
> OK, are these fixed in 9.4.2 or would the same failure happen in 9.4.3?

The fixes are not yet in any released branch, hence the rush to get
these out.

-- 
Álvaro Herrerahttp://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] [CORE] Restore-reliability mode

[Stephen Frost]

David,

* David G. Johnston (david.g.johns...@gmail.com) wrote:
> On Mon, Jun 8, 2015 at 12:03 PM, Claudio Freire 
> wrote:
> > Just *when* is git ready for testing? You don't know from the outside.
> >
> > I do lurk here a lot and still am unsure quite often.
> >
> > Even simply releasing an alpha *tarball* would be useful enough. What
> > is needed is the signal to test, rather than a fully-built package.
> >
> >
> IIUC the master branch is always ready for testing.
> 
> I do not think the project cares whether everyone is testing the exact
> same codebase; as long as test findings include the relevant commit hash
> the results will be informative.

For my 2c, I do believe it's useful for projects which are based on PG
or which work with PG to have a 'alpha1' tag to refer to.  Asking users
to test with git hash XYZABC isn't great.  Getting more users of
applications which use PG to do testing is, in my view at least, a great
way to improve our test coverage and I do think having an alpha will
help with that.

That said, I'm not pushing to have one released this week or before
PGCon or any such- let's get the back-branch releases dealt with and
then we can get an alpha out.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Bruce Momjian]

On Mon, Jun  8, 2015 at 01:53:42PM -0300, Alvaro Herrera wrote:
> * people with the wrong oldestMulti setting in pg_control (which would
> be due to a buggy pg_upgrade being used long ago) will be unable to
> start if they upgrade to 9.3.7 or 9.3.8.  A solution for them would be
> to downgrade to 9.3.6.  We had reports of this problem starting just a
> couple of days after we released 9.4.2, I think.
> 
> * We had a customer unable to refresh their base backups once they
> upgraded to 9.3.7; taking a new base backup would fail with a very
> similar error to those above (except no buggy pg_upgrade was involved).
> They seem to have gotten from under that problem by removing from
> crontab a script that ran whole-table vacuuming more frequently than
> with default settings.  Their data is 3 TB in size, so the basebackup
> takes long enough that multixact truncations occured while the base
> backups were running, every time, so they were unrestorable.
> 
> (Actually I just checked and it seems they haven't verified that they
> can take a new base backup -- the new one is still running.)
> 
> Anyway my point is that for some guys these bugs are pretty critical.

OK, thanks for the summary.  I assume they would still have problems
with 9.4.3.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


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

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Bruce Momjian]

On Mon, Jun  8, 2015 at 12:39:24PM -0400, Robert Haas wrote:
> On Mon, Jun 8, 2015 at 12:36 PM, Bruce Momjian  wrote:
> > Yeah, I think if we needed this out in an emergency, we would do it, but
> > based on the volume of recent releases, it would be hard.  Are we seeing
> > user reports of failures even on the newest released versions, or are
> > these preventive fixes?
> 
> User reports of failures.  See the thread about upgrading from 9.4.1
> to 9.4.2 and having the server *fail to start*.

OK, are these fixed in 9.4.2 or would the same failure happen in 9.4.3?

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


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

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Alvaro Herrera]

Bruce Momjian wrote:
> On Mon, Jun  8, 2015 at 11:40:43AM -0400, Tom Lane wrote:
> > Robert Haas  writes:
> > > So, when shall we do all of this releasing?  It seems like we could do
> > > stage-one of the multixact fixing this week, and then figure out how
> > > to do the other stuff after PGCon.  Alternatively, we can let the
> > > latest round of changes that are already in the tree settle until
> > > after PGCon, and plan a release for stage-one of the multixact fixing
> > > then.  Whichever we pick, we then need to figure out the timetable for
> > > the rest of it.
> > 
> > I think we've already basically missed the window for releases this week.
> > Not that we couldn't physically do it, but that we normally give the
> > packagers more than one day's notice.  (There's also the fact that we've
> > already asked them to do two releases in the past three weeks.)
> 
> Yeah, I think if we needed this out in an emergency, we would do it, but
> based on the volume of recent releases, it would be hard.  Are we seeing
> user reports of failures even on the newest released versions, or are
> these preventive fixes?

* people with the wrong oldestMulti setting in pg_control (which would
be due to a buggy pg_upgrade being used long ago) will be unable to
start if they upgrade to 9.3.7 or 9.3.8.  A solution for them would be
to downgrade to 9.3.6.  We had reports of this problem starting just a
couple of days after we released 9.4.2, I think.

* We had a customer unable to refresh their base backups once they
upgraded to 9.3.7; taking a new base backup would fail with a very
similar error to those above (except no buggy pg_upgrade was involved).
They seem to have gotten from under that problem by removing from
crontab a script that ran whole-table vacuuming more frequently than
with default settings.  Their data is 3 TB in size, so the basebackup
takes long enough that multixact truncations occured while the base
backups were running, every time, so they were unrestorable.

(Actually I just checked and it seems they haven't verified that they
can take a new base backup -- the new one is still running.)

Anyway my point is that for some guys these bugs are pretty critical.

-- 
Álvaro Herrerahttp://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] pg_stat_*_columns?

[Robert Haas]

On Fri, Jun 5, 2015 at 7:51 AM, Joel Jacobson  wrote:
> Would others find it useful to see per column statistics about accesses to
> specific columns?

A probably serious and maybe not entirely obvious problem with this is
that it would increase the amount of statistical information we keep
by a pretty large multiple.  How many columns do you have in a typical
table?  20-30?  That's a lot of data for a statistics collector that,
regrettably, is already overloaded.

-- 
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] [CORE] Restore-reliability mode

[Andres Freund]

On 2015-06-08 12:16:34 -0400, David G. Johnston wrote:
> ​IIUC the master branch is always ready for testing.

I don't really think so. For one we often find bugs ourselves quite
quickly.

But more importantly, I'd much rather have users use their precious (and
thus limited!) time to test when the set of features (not every detail
of a feature) is mostly set in stone. There's not much point in doing
in-depth testing before that. Similarly it's not particularly worthwhile
to test while the buildfarm still shows failures on common platforms.

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: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Robert Haas]

On Mon, Jun 8, 2015 at 12:36 PM, Bruce Momjian  wrote:
> Yeah, I think if we needed this out in an emergency, we would do it, but
> based on the volume of recent releases, it would be hard.  Are we seeing
> user reports of failures even on the newest released versions, or are
> these preventive fixes?

User reports of failures.  See the thread about upgrading from 9.4.1
to 9.4.2 and having the server *fail to start*.

-- 
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] [GENERAL] 9.4.1 -> 9.4.2 problem: could not access status of transaction 1

[Andres Freund]

On 2015-06-08 15:15:04 +0200, Andres Freund wrote:
> 1) the autovacuum trigger logic isn't perfect yet. I.e. especially with
>   autovacuum=off you can get into situations where emergency vacuums
>   aren't started when necessary. This is particularly likely to happen
>   if either very large multixacts are used, or if the server has been
>   shut down while emergency autovacuum where happening. No corruption
>   ensues, but it's not easy to get out of.

A first version to address this problem can be found appended to this
email.

Basically it does:
* Whenever more than MULTIXACT_MEMBER_SAFE_THRESHOLD are used, signal
  autovacuum once per members segment
* For both members and offsets, once hitting the hard limits, signal
  autovacuum everytime. Otherwise we loose the information when
  restarting the database, or when autovac is killed. I ran into this a
  bunch of times while testing.

Regards,

Andres
>From 9949d8ce4b69b4fd693da08d8e1854fd259a33a9 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Mon, 8 Jun 2015 13:41:42 +0200
Subject: [PATCH] Improve multixact emergency autovacuum logic.

Previously autovacuum was not necessarily triggered if space in the
members slru got tight. The first problem was that the signalling was
tied to values in the offsets slru, but members can advance much
faster. Thats especially a problem if old sessions had been around that
previously prevented the multixact horizon to increase. Secondly the
skipping logic doesn't work if the database was restarted after
autovacuum was triggered - that knowledge is not preserved across
restart. This is especially a problem because it's a common
panic-reaction to restart the database if it gets slow to
anti-wraparound vacuums.

Fix the first problem by separating the logic for members from
offsets. Trigger autovacuum whenever a multixact crosses a segment
boundary, as the current member offset increases in irregular values, so
we can't use a simple modulo logic as for offsets.  Add a stopgap for
the second problem, by signalling autovacuum whenver ERRORing out
because of boundaries.

Backpatch into 9.3, where it became more likely that multixacts wrap
around.
---
 src/backend/access/transam/multixact.c | 61 +-
 1 file changed, 45 insertions(+), 16 deletions(-)

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index d3336a8..3bc170d 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -980,10 +980,7 @@ GetNewMultiXactId(int nmembers, MultiXactOffset *offset)
 	 * Note these are pretty much the same protections in GetNewTransactionId.
 	 *--
 	 */
-	if (!MultiXactIdPrecedes(result, MultiXactState->multiVacLimit) ||
-		!MultiXactState->oldestOffsetKnown ||
-		(MultiXactState->nextOffset - MultiXactState->oldestOffset
-		 > MULTIXACT_MEMBER_SAFE_THRESHOLD))
+	if (!MultiXactIdPrecedes(result, MultiXactState->multiVacLimit))
 	{
 		/*
 		 * For safety's sake, we release MultiXactGenLock while sending
@@ -999,19 +996,17 @@ GetNewMultiXactId(int nmembers, MultiXactOffset *offset)
 
 		LWLockRelease(MultiXactGenLock);
 
-		/*
-		 * To avoid swamping the postmaster with signals, we issue the autovac
-		 * request only once per 64K multis generated.  This still gives
-		 * plenty of chances before we get into real trouble.
-		 */
-		if (IsUnderPostmaster && (result % 65536) == 0)
-			SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_LAUNCHER);
-
 		if (IsUnderPostmaster &&
 			!MultiXactIdPrecedes(result, multiStopLimit))
 		{
 			char	   *oldest_datname = get_database_name(oldest_datoid);
 
+			/*
+			 * Immediately kick autovacuum into action as we're already
+			 * in ERROR territory.
+			 */
+			SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_LAUNCHER);
+
 			/* complain even if that DB has disappeared */
 			if (oldest_datname)
 ereport(ERROR,
@@ -1032,6 +1027,14 @@ GetNewMultiXactId(int nmembers, MultiXactOffset *offset)
 		{
 			char	   *oldest_datname = get_database_name(oldest_datoid);
 
+			/*
+			 * To avoid swamping the postmaster with signals, we issue the autovac
+			 * request only once per 64K multis generated.  This still gives
+			 * plenty of chances before we get into real trouble.
+			 */
+			if (IsUnderPostmaster && (result % 65536) == 0)
+SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_LAUNCHER);
+
 			/* complain even if that DB has disappeared */
 			if (oldest_datname)
 ereport(WARNING,
@@ -1099,6 +1102,10 @@ GetNewMultiXactId(int nmembers, MultiXactOffset *offset)
 	if (MultiXactState->offsetStopLimitKnown &&
 		MultiXactOffsetWouldWrap(MultiXactState->offsetStopLimit, nextOffset,
  nmembers))
+	{
+		/* see comment in the corresponding offsets wraparound case */
+		SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_LAUNCHER);
+
 		ereport(ERROR,
 (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
  errmsg("multixact \"members\" limit exceeded"),
@@ -1109,10 +1116,32 @@ GetNewMultiXactId(int

Re: [CORE] [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Bruce Momjian]

On Mon, Jun  8, 2015 at 11:40:43AM -0400, Tom Lane wrote:
> Robert Haas  writes:
> > So, when shall we do all of this releasing?  It seems like we could do
> > stage-one of the multixact fixing this week, and then figure out how
> > to do the other stuff after PGCon.  Alternatively, we can let the
> > latest round of changes that are already in the tree settle until
> > after PGCon, and plan a release for stage-one of the multixact fixing
> > then.  Whichever we pick, we then need to figure out the timetable for
> > the rest of it.
> 
> I think we've already basically missed the window for releases this week.
> Not that we couldn't physically do it, but that we normally give the
> packagers more than one day's notice.  (There's also the fact that we've
> already asked them to do two releases in the past three weeks.)

Yeah, I think if we needed this out in an emergency, we would do it, but
based on the volume of recent releases, it would be hard.  Are we seeing
user reports of failures even on the newest released versions, or are
these preventive fixes?

> I propose that we plan for back-branch releases the week after PGCon
> (wrap on Monday June 22), and 9.5alpha1 the week after that (wrap on
> Monday June 29).  If there's a need for an additional round of back-branch
> releases shortly thereafter, we'll deal with that as needed.

I am working on the 9.5 release notes so will be done long before that
date.  I will finish sooner so we can do the week-long release notes
feedback session.  :-)

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
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] [CORE] Restore-reliability mode

[David G. Johnston]

On Mon, Jun 8, 2015 at 12:14 PM, Geoff Winkless  wrote:

> On 8 June 2015 at 17:03, Claudio Freire  wrote:
>
>> It's not about the 5 minutes of compile time, it's about the signalling.
>>
>> Just *when* is git ready for testing? You don't know from the outside.
>>
>> I do lurk here a lot and still am unsure quite often.
>>
>> Even simply releasing an alpha *tarball* would be useful enough. What
>> is needed is the signal to test, rather than a fully-built package.
>>
>
> ​I can see that, and can absolutely get behind the idea of a nightly being
> flagged as an alpha, since it should involve next to no developer time.
>
>
​Nightly where?  This is an international community.

The tip of the master branch is the current "alpha" - so the question is
whether a tar bundle should be provided instead of asking people to simply
keep their Git clone up-to-date.  These both have the flaw of excluding
people who would test the application if it could simply be installed like
any other package on their system.  But I'm not seeing where there would be
a huge group of people who would test an automatically generated source
tar-ball but would not be willing to use Git.  Or are we talking about a
non-source tar-ball?

Maybe packagers could be convinced to bundle up the master branch on a
monthly basis and simply call it Master-SNAPSHOT.  No alpha, no beta, no
version number.  I've never packaged before so I don't know but while the
project should encourage this as things currently standard the core project
is doing its job by ensuring that the tip of master is always in a usable
state.

Or, whenever a new patch release goes out packagers can also bundle up the
current master at the same time.

David J.

Re: [HACKERS] [CORE] Restore-reliability mode

[David G. Johnston]

On Mon, Jun 8, 2015 at 12:03 PM, Claudio Freire 
wrote:

> Just *when* is git ready for testing? You don't know from the outside.
>
> I do lurk here a lot and still am unsure quite often.
>
> Even simply releasing an alpha *tarball* would be useful enough. What
> is needed is the signal to test, rather than a fully-built package.
>
>
​IIUC the master branch is always ready for testing.

​I do not think the project cares whether everyone is testing the exact
same codebase; as long as test findings include the relevant commit hash
the results will be informative.

David J.​

Re: [HACKERS] [CORE] Restore-reliability mode

[Geoff Winkless]

On 8 June 2015 at 17:03, Claudio Freire  wrote:

> It's not about the 5 minutes of compile time, it's about the signalling.
>
> Just *when* is git ready for testing? You don't know from the outside.
>
> I do lurk here a lot and still am unsure quite often.
>
> Even simply releasing an alpha *tarball* would be useful enough. What
> is needed is the signal to test, rather than a fully-built package.
>

​I can see that, and can absolutely get behind the idea of a nightly being
flagged as an alpha, since it should involve next to no developer time.

I may be overestimating the amount of time that goes towards producing a
release; the fact that the full-on alpha releases were stopped did imply to
me that it's not insignificant.

Geoff​

Re: [HACKERS] Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file

[Robert Haas]

On Mon, Jun 8, 2015 at 11:16 AM, Amit Kapila  wrote:
> I have to retry that operation, but for me unlink hasn't deleted
> the file on Windows, may be I am not doing properly, but in
> anycase why we want to throw error for such a case, why
> can't we just ignore and create a symlink with the same name.
>
>> I realize our existing code just more or less assumes that that it's a
>> symlink. I think we've probably been a bit careless there.
>
> I agree with you that deleting unrelated file with the same name as
> symlink is not the right thing to do, but not sure throwing error for
> such a case is better either.

Why not?  I think that if we encounter some sort of situation that we
think should never happen, throwing an error is exactly what we
*should* do.  Particularly when it comes to things like removing
files, it is very dangerous for the database to proceed if the
situation is not as expected.  We should only remove things if we are
quite sure that removing them is the right thing to do.

-- 
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] [CORE] Restore-reliability mode

[Claudio Freire]

On Mon, Jun 8, 2015 at 12:22 PM, Geoff Winkless  wrote:
> On 8 June 2015 at 16:01, Robert Haas  wrote:
>>
>> On Mon, Jun 8, 2015 at 9:21 AM, Geoff Winkless 
>> wrote:
>> > Wow! I never knew there were all these people out there who would be
>> > rushing
>> > to help test if only the PG developers released alpha versions. It's
>> > funny
>> > how they never used to do it when those alphas were done.
>>
>> That's probably overplaying your hand a little bit (and it sounds a
>> bit catty, too).
>
>
> I agree. The responses I had written yesterday but didn't send were much
> worse.
>
> Mainly because I think it's quite an attitude to take that open-source
> developers should put extra time into building RPMs of development versions
> rather than testers waiting 5 minutes while their machines compile.
> Ohmygosh, you have to rpm install a bunch of -devel stuff? What a massive
> hardship.

It's not about the 5 minutes of compile time, it's about the signalling.

Just *when* is git ready for testing? You don't know from the outside.

I do lurk here a lot and still am unsure quite often.

Even simply releasing an alpha *tarball* would be useful enough. What
is needed is the signal to test, rather than a fully-built package.


-- 
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] Initializing initFileRelationIds list via write is unsafe

[Robert Haas]

On Sat, Jun 6, 2015 at 11:51 PM, Tom Lane  wrote:
> This suggests that CLOBBER_CACHE_ALWAYS is actually missing a pretty
> large part of the cache behavioral space.  Maybe we should devise some
> sort of "CLOBBER_CACHE_RANDOMLY" option that would inject cache flush
> events more selectively, perhaps only once every thousand opportunities
> or so.  And perhaps not only full cache reset events, though I confess
> to not being sure what that ought to look like.

It should probably be done pseudo-randomly, but otherwise, +1.

-- 
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] CREATE POLICY and RETURNING

[Stephen Frost]

* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 6 June 2015 at 22:16, Peter Geoghegan  wrote:
> > On Fri, Oct 17, 2014 at 5:34 AM, Andres Freund  
> > wrote:
> >> On 2014-10-17 14:57:03 +0800, Craig Ringer wrote:
> >>> On 10/17/2014 02:49 AM, Robert Haas wrote:
> >>> > I think you could probably make the DELETE policy control what can get
> >>> > deleted, but then have the SELECT policy further filter what gets
> >>> > returned.
> >>>
> >>> That seems like the worst of both worlds to me.
> >>>
> >>> Suddenly DELETE ... RETURNING might delete more rows than it reports a
> >>> resultset for. As well as being potentially dangerous for people using
> >>> it in wCTEs, etc, to me that's the most astonishing possible outcome of 
> >>> all.
> >>>
> >>> I'd be much happier with even:
> >>>
> >>>   ERROR: RETURNING not permitted with SELECT row-security policy
> >>
> >> FWIW, that doesn't sound acceptable to me.
> >
> > This is more or less what ended up happening with UPSERT and USING
> > security barrier quals on UPDATE/ALL policies. Realistically, the
> > large majority of use cases don't involve a user being able to
> > INSERT/DELETE tuples, but not SELECT them, and those that do should
> > not be surprised to have a RETURNING fail (it's an odd enough union of
> > different features that this seems acceptable to me).
> >
> > Like Fujii, I think that RETURNING with RLS should not get to avoid
> > SELECT policies. I agree with the concern about not seeing affected
> > rows with a DELETE (which, as I said, is very similar to UPSERT +
> > WCO_RLS_CONFLICT_CHECK policies), so an error seems like the only
> > alternative.
> >
> > The argument against not requiring SELECT *column* privilege on the
> > EXCLUDED.* pseudo relation for UPSERT might have been: "well, what can
> > be the harm of allowing the user to see what they themselves might
> > have inserted?". But that would have been a bad argument then had
> > anyone made it, because RETURNING with a (vanilla) INSERT requires
> > SELECT privilege, and that's also what the user then actually inserted
> > (as distinct from what the user *would have* inserted had the insert
> > path been taking, representing as the EXCLUDED.* pseudo relation --
> > for security purposes, ISTM that this is really no distinction at
> > all). Consider before row insert triggers that can modify EXCLUDED.*
> > tuples in a privileged way.
> >
> > So, the only logical reason that INSERT with RETURNING requires SELECT
> > column privilege that I can see is that a before row INSERT trigger
> > could modify the tuple inserted in a way that the inserter role should
> > not know the details of. This long standing convention was reason
> > enough to mandate that SELECT column privilege be required for the
> > EXCLUDED.* pseudo relation for UPSERT. And so, I think it isn't too
> > much of a jump to also say that we should do the same for RLS (for
> > INSERTs for the reason I state, but also for UPDATEs and DELETEs for a
> > far more obvious reason: the *existing* tuple can be projected, and
> > the updater/deleter might well have no business seeing its contents).
> >
> > In short: I think we should be tracking a new WCOKind (perhaps
> > WCO_RLS_RETURNING_CHECK?), that independently holds the security
> > barrier quals as WCO-style checks when that's recognized as being
> > necessary. For INSERT, these WCOs must be enforced against the target
> > tuple projected by RETURNING. For UPDATEs and DELETEs, FROM/USING
> > relations must also have SELECT privilege enforced against the
> > projected target tuple, as well as the non-target relation --
> > apparently the latter isn't currently happening, although Dean has
> > tried to address this with his recent patch [1]. That is, even
> > non-target relations (UPDATE ... FROM relations, or DELETE ... USING
> > relations) do not have SELECT policy enforcement, but rather have
> > UPDATE or DELETE policy enforcement only. I must admit that I was
> > rather surprised at that; it has to be a bug.
> >
> 
> Yes, I think a query with a RETURNING clause ought to either succeed
> and return everything the user asked for, or error out if some/all of
> the data isn't visible to the user according to SELECT policies in
> effect. I think not applying the SELECT policies is wrong, and
> returning a portion of the data updated would just be confusing.

I definitely don't like the idea of returning a portion of the data in a
RETURNING clause.  Returning an error if the RETURNING clause ends up
not passing the SELECT policy is an interesting idea, but I do have
doubts about how often that will be a useful exercise.  Another issue to
note is that, currently, SELECT policies never cause errors to be
returned, and this would change that.

There was discussion about a VISIBILITY policy instead of a SELECT
policy at one point.  What I think we're seeing here is confusion about
the various policies which exist, because the USING policy of an UPDATE
is precisely its VISIBILIT

Re: [HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Tom Lane]

Robert Haas  writes:
> So, when shall we do all of this releasing?  It seems like we could do
> stage-one of the multixact fixing this week, and then figure out how
> to do the other stuff after PGCon.  Alternatively, we can let the
> latest round of changes that are already in the tree settle until
> after PGCon, and plan a release for stage-one of the multixact fixing
> then.  Whichever we pick, we then need to figure out the timetable for
> the rest of it.

I think we've already basically missed the window for releases this week.
Not that we couldn't physically do it, but that we normally give the
packagers more than one day's notice.  (There's also the fact that we've
already asked them to do two releases in the past three weeks.)

I propose that we plan for back-branch releases the week after PGCon
(wrap on Monday June 22), and 9.5alpha1 the week after that (wrap on
Monday June 29).  If there's a need for an additional round of back-branch
releases shortly thereafter, we'll deal with that as needed.

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] [CORE] Restore-reliability mode

[Geoff Winkless]

On 8 June 2015 at 16:01, Robert Haas  wrote:

> On Mon, Jun 8, 2015 at 9:21 AM, Geoff Winkless 
> wrote:
> > Wow! I never knew there were all these people out there who would be
> rushing
> > to help test if only the PG developers released alpha versions. It's
> funny
> > how they never used to do it when those alphas were done.
>
> That's probably overplaying your hand a little bit (and it sounds a
> bit catty, too).


​I agree. The responses I had written yesterday but didn't send were much
worse.

Mainly because I think it's quite an attitude to take that open-source
developers should put extra time into building RPMs of development versions
rather than testers waiting 5 minutes while their machines compile.
Ohmygosh, you have to rpm install a bunch of -devel stuff? What a massive
hardship.

​
​
On 8 June 2015 at 16:06, Joshua D. Drake  wrote:

> ​​
> The type of responses you are providing on this thread are not warranted.
>

I got people appearing completely insulted at my remarks and telling me
that if only they could run the alpha they would provide testing, so I
pointed out how easy it is to install the nightly from source and then they
tell me that actually compiling is far too difficult and complicated, and
that there are loads of clients who would run these nightlies if they had
RPMS...

If I truly believed that such an RPM would produce useful testing, I would
spend some of my own time building a setup to produce those RPMs myself and
post here publicising them, at which point we would have a huge number of
useful and productive test reports. Any one of the people telling me that
I'm wrong could easily do the same, but so far none has.

I'm not harping on because I want to make people feel bad, I'm harping on
because I don't want to see beta (and final) releases pushed back further
because of a bad compromise, and I believe that that will happen. I
apologise that I've clearly upset some people but they all have a very easy
route to prove me wrong, and I'll be happy to admit my error.
​
​Geoff​

Re: [HACKERS] [CORE] Restore-reliability mode

[Petr Jelinek]

On Mon, Jun 8, 2015 at 5:01 , Robert Haas  wrote:
On Mon, Jun 8, 2015 at 9:21 AM, Geoff Winkless  
wrote:
 Wow! I never knew there were all these people out there who would 
be rushing
 to help test if only the PG developers released alpha versions. 
It's funny

 how they never used to do it when those alphas were done.


That's probably overplaying your hand a little bit (and it sounds a
bit catty, too).  Some testing got done and it had some value.  It
just wasn't enough to make Peter feel like it was worthwhile.  That
doesn't mean that no testing got done and that it had no value, or
that the same thing would happen this time.  I'm as skeptical about
this whole rush-out-an-alpha business as anyone, but I think that
skepticism has to yield to contrary evidence, and people saying "I
would test if..." is legitimate contrary evidence.



Agreed.

To get back to the point, I think the problem with original alphas was 
that they were after CF snapshots, not something that represented the 
final release.


I do think that proper alpha/beta release is signal for several 
companies (I do know some that do testing once beta gets out) to do 
testing as it does indeed say that we are releasing something that is 
close in functionality to the final release.


Also the packages are really important, there are enough companies that 
don't install development packages to servers at all so it's not just 
compile and run for them, they have to move it over to other machines, 
etc. We should be lowering the barrier to user based testing as much as 
possible and doing alpha with packages is exactly how we do that.


IMHO the only real discussion here is if current 9.5 is ready for user 
testing and FWIW I thin it is.



--
Petr Jelinek  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] Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file

[Amit Kapila]

On Mon, Jun 8, 2015 at 6:39 PM, Andrew Dunstan  wrote:

>
> On 06/08/2015 12:08 AM, Amit Kapila wrote:
>
>> How about if it is just a flat file with same name as tablespace link,
>> why we want to give error for that case?  I think now it just don't do
>> anything with that file (unlink will fail with ENOENT and it will be
>> ignored, atleast thats the way currently it behaves in Windows) and
>> create a separate symlink with same name which seems okay to
>> me and in the change proposed by you it will give error, do you see
>> any reason for doing so?
>>
>>
>>
>
> This is surely wrong. unlink won't fail with ENOENT if the file is
> present; ENOENT means that the file is NOT present. It will succeed if the
> file is present, which is exactly what I'm saying is wrong.
>

I have to retry that operation, but for me unlink hasn't deleted
the file on Windows, may be I am not doing properly, but in
anycase why we want to throw error for such a case, why
can't we just ignore and create a symlink with the same name.


> I realize our existing code just more or less assumes that that it's a
> symlink. I think we've probably been a bit careless there.
>

I agree with you that deleting unrelated file with the same name as
symlink is not the right thing to do, but not sure throwing error for
such a case is better either.


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

Re: [HACKERS] bugfix: incomplete implementation of errhidecontext

[Pavel Stehule]

2015-06-08 16:49 GMT+02:00 Andres Freund :

> On 2015-06-08 14:44:53 +, Jeevan Chalke wrote:
> > The following review has been posted through the commitfest application:
> > make installcheck-world:  tested, passed
> > Implements feature:   tested, passed
> > Spec compliant:   tested, passed
> > Documentation:tested, passed
> >
> > This is trivial bug fix in the area of hiding error context.
> >
> > I observed that there are two places from which we are calling this
> function
> > to hide the context in log messages. Those were broken.
>
> Broken in which sense? They did prevent stuff to go from the server log?
>
> I'm not convinced that hiding stuff from the client is really
> necessarily the same as hiding it from the server log. We e.g. always
> send the verbose log to the client, even if we only send the terse
> version to the server log.  I don't mind adjusting things for
> errhidecontext(), but it's not "just a bug".
>

Hard to say if it is bug or not - actually it is not consistent - the name
signalize so context will not be used - and there are no any other
possibility to specify if it should be only for client side or for all.

I don't would to do more complex than it is - just when is some exception
marked as "hide context" I expect, so context will not be shown everywhere.
Probably we should not to introduce function

errreallyhiddencontext()  :)

Regards

Pavel


>
> 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] [CORE] Restore-reliability mode

[Joshua D. Drake]

On 06/08/2015 06:21 AM, Geoff Winkless wrote:



Wow! I never knew there were all these people out there who would be
rushing to help test if only the PG developers released alpha versions.
It's funny how they never used to do it when those alphas were done.


The type of responses you are providing on this thread are not warranted.

JD

--
The most kicking donkey PostgreSQL Infrastructure company in existence.
The oldest, the most experienced, the consulting company to the stars.
Command Prompt, Inc. http://www.commandprompt.com/ +1 -503-667-4564 -
24x7 - 365 - Proactive and Managed Professional 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] [CORE] Restore-reliability mode

[Robert Haas]

On Mon, Jun 8, 2015 at 9:21 AM, Geoff Winkless  wrote:
> Wow! I never knew there were all these people out there who would be rushing
> to help test if only the PG developers released alpha versions. It's funny
> how they never used to do it when those alphas were done.

That's probably overplaying your hand a little bit (and it sounds a
bit catty, too).  Some testing got done and it had some value.  It
just wasn't enough to make Peter feel like it was worthwhile.  That
doesn't mean that no testing got done and that it had no value, or
that the same thing would happen this time.  I'm as skeptical about
this whole rush-out-an-alpha business as anyone, but I think that
skepticism has to yield to contrary evidence, and people saying "I
would test if..." is legitimate contrary evidence.

-- 
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] bugfix: incomplete implementation of errhidecontext

[Andres Freund]

On 2015-06-08 14:44:53 +, Jeevan Chalke wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world:  tested, passed
> Implements feature:   tested, passed
> Spec compliant:   tested, passed
> Documentation:tested, passed
> 
> This is trivial bug fix in the area of hiding error context.
> 
> I observed that there are two places from which we are calling this function
> to hide the context in log messages. Those were broken.

Broken in which sense? They did prevent stuff to go from the server log?

I'm not convinced that hiding stuff from the client is really
necessarily the same as hiding it from the server log. We e.g. always
send the verbose log to the client, even if we only send the terse
version to the server log.  I don't mind adjusting things for
errhidecontext(), but it's not "just a bug".

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] bugfix: incomplete implementation of errhidecontext

[Jeevan Chalke]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

This is trivial bug fix in the area of hiding error context.

I observed that there are two places from which we are calling this function
to hide the context in log messages. Those were broken.

This patch fixes those.

So good to go in.

The new status of this patch is: Ready for Committer


-- 
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: Add pg_audit, an auditing extension

[Noah Misch]

Ian, Abhijit, and David,

My condemnation of the pg_audit commits probably hurt you as the feature's
authors.  I am sorry for that.  Your code was better than most "Ready for
Committer" code, and I hope you submit more patches in the future.

Regards,
nm


-- 
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] Collection of memory leaks for ECPG driver

[Michael Paquier]

On Mon, Jun 8, 2015 at 9:22 PM, Michael Meskes wrote:
> On Mon, Jun 08, 2015 at 01:57:32PM +0900, Michael Paquier wrote:
>> diff --git a/src/interfaces/ecpg/compatlib/informix.c 
>> b/src/interfaces/ecpg/compatlib/informix.c
>> index d6de3ea..c1e3dfb 100644
>> --- a/src/interfaces/ecpg/compatlib/informix.c
>> +++ b/src/interfaces/ecpg/compatlib/informix.c
>> @@ -672,6 +672,7 @@ intoasc(interval * i, char *str)
>>   if (!str)
>>   return -errno;
>>
>> + free(str);
>>   return 0;
>>  }
>
> This function never worked it seems, we need to use a temp string, copy it to 
> str and then free the temp one.

And the caller needs to be sure that str actually allocates enough
space.. That's not directly ECPG's business, still it feels
uncomfortable. I fixed it with the attached.

>> diff --git a/src/interfaces/ecpg/ecpglib/connect.c 
>> b/src/interfaces/ecpg/ecpglib/connect.c
>> index 55c5680..12f445d 100644
>> --- a/src/interfaces/ecpg/ecpglib/connect.c
>> +++ b/src/interfaces/ecpg/ecpglib/connect.c
>> @@ -298,7 +298,8 @@ ECPGconnect(int lineno, int c, const char *name, const 
>> char *user, const char *p
>>   envname = getenv("PG_DBPATH");
>>   if (envname)
>>   {
>> - ecpg_free(dbname);
>> + if (dbname)
>> + ecpg_free(dbname);
>>   dbname = ecpg_strdup(envname, lineno);
>>   }
>
> This is superfluous, at least with glibc. free()'s manpage clearly says "If
> ptr is NULL, no operation is performed.", so why should we check again? Or do
> we have architectures where free(NULL) is not a noop?

The world is full of surprises, and even if free(NULL) is a noop on
modern platforms, I would rather take it defensively and check for a
NULL pointer as Postgres supports many platforms. Other code paths
tend to do already so, per se for example ECPGconnect.

>> diff --git a/src/interfaces/ecpg/preproc/descriptor.c 
>> b/src/interfaces/ecpg/preproc/descriptor.c
>> index 053a7af..ebd95d3 100644
>> --- a/src/interfaces/ecpg/preproc/descriptor.c
>> +++ b/src/interfaces/ecpg/preproc/descriptor.c
>
> Do we really have to worry about these in a short running application like a 
> precompiler, particularly if they add more than a simple free() command?

Perhaps I am overdoing it. I would have them for correctness (some
other code paths do so) but if you think that the impact is minimal
there then I am fine to not modify descriptor.c.

> But then, you already did the work, so why not. Anyway, please tell me if you 
> want to update the patch or if I should commit whatever is clear already.

A new patch is attached.
Regards,
-- 
Michael
diff --git a/src/interfaces/ecpg/compatlib/informix.c b/src/interfaces/ecpg/compatlib/informix.c
index d6de3ea..8d81c83 100644
--- a/src/interfaces/ecpg/compatlib/informix.c
+++ b/src/interfaces/ecpg/compatlib/informix.c
@@ -666,12 +666,16 @@ dttofmtasc(timestamp * ts, char *output, int str_len, char *fmtstr)
 int
 intoasc(interval * i, char *str)
 {
+	char *tmp;
+
 	errno = 0;
-	str = PGTYPESinterval_to_asc(i);
+	tmp = PGTYPESinterval_to_asc(i);
 
-	if (!str)
+	if (!tmp)
 		return -errno;
 
+	memcpy(str, tmp, strlen(tmp));
+	free(tmp);
 	return 0;
 }
 
diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 55c5680..12f445d 100644
--- a/src/interfaces/ecpg/ecpglib/connect.c
+++ b/src/interfaces/ecpg/ecpglib/connect.c
@@ -298,7 +298,8 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 		envname = getenv("PG_DBPATH");
 		if (envname)
 		{
-			ecpg_free(dbname);
+			if (dbname)
+ecpg_free(dbname);
 			dbname = ecpg_strdup(envname, lineno);
 		}
 
@@ -314,14 +315,19 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 	/* check if the identifier is unique */
 	if (ecpg_get_connection(connection_name))
 	{
-		ecpg_free(dbname);
+		if (dbname)
+			ecpg_free(dbname);
 		ecpg_log("ECPGconnect: connection identifier %s is already in use\n",
  connection_name);
 		return false;
 	}
 
 	if ((this = (struct connection *) ecpg_alloc(sizeof(struct connection), lineno)) == NULL)
+	{
+		if (dbname)
+			ecpg_free(dbname);
 		return false;
+	}
 
 	if (dbname != NULL)
 	{
diff --git a/src/interfaces/ecpg/preproc/descriptor.c b/src/interfaces/ecpg/preproc/descriptor.c
index 053a7af..ebd95d3 100644
--- a/src/interfaces/ecpg/preproc/descriptor.c
+++ b/src/interfaces/ecpg/preproc/descriptor.c
@@ -175,6 +175,7 @@ output_get_descr(char *desc_name, char *index)
 	for (results = assignments; results != NULL; results = results->next)
 	{
 		const struct variable *v = find_variable(results->variable);
+		char *str_zero = mm_strdup("0");
 
 		switch (results->value)
 		{
@@ -188,7 +189,8 @@ output_get_descr(char *desc_name, char *index)
 break;
 		}
 		fprintf(yyout, "%s,", get_dtype(results->value));
-		ECPGdump_a_type(yyout, v->name, v->type, v->brace_level,

Re: [HACKERS] Memory leak fixes for pg_dump, pg_dumpall, initdb and pg_upgrade

[Michael Paquier]

On Mon, Jun 8, 2015 at 10:26 PM, Michael Paquier
 wrote:
> On Mon, Jun 8, 2015 at 3:48 PM, Michael Paquier
>  wrote:
>> Hi all,
>>
>> Please find attached a set of fixes for a couple of things in src/bin:
>> - pg_dump/pg_dumpall:
>> -- getFormattedTypeName, convertTSFunction and myFormatType return
>> strdup'd results that are never free'd.
>> -- convertTSFunction returns const char. I fail to see the point of
>> that... In my opinion we are fine with just returning a char pointer,
>> which is strdup'd so as it can be freed by the caller.
>> - initdb's and pg_regress' use getaddrinfo, but do not free the
>> returned result with freeaddrinfo().
>> - Coverity noticed on the way some leaked memory in pg_upgrade's
>> equivalent_locale().
>>
>> Those issues have been mostly spotted by Coverity, I may have spotted
>> some of them while looking at similar code paths... In any case that's
>> Coverity's win ;)
>
> Attached are new patches, I simplified the use of free in the fixes of
> pg_dumpall.

Please ignore those versions, I am just too sleepy...
-- 
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] Memory leak fixes for pg_dump, pg_dumpall, initdb and pg_upgrade

[Michael Paquier]

On Mon, Jun 8, 2015 at 3:48 PM, Michael Paquier
 wrote:
> Hi all,
>
> Please find attached a set of fixes for a couple of things in src/bin:
> - pg_dump/pg_dumpall:
> -- getFormattedTypeName, convertTSFunction and myFormatType return
> strdup'd results that are never free'd.
> -- convertTSFunction returns const char. I fail to see the point of
> that... In my opinion we are fine with just returning a char pointer,
> which is strdup'd so as it can be freed by the caller.
> - initdb's and pg_regress' use getaddrinfo, but do not free the
> returned result with freeaddrinfo().
> - Coverity noticed on the way some leaked memory in pg_upgrade's
> equivalent_locale().
>
> Those issues have been mostly spotted by Coverity, I may have spotted
> some of them while looking at similar code paths... In any case that's
> Coverity's win ;)

Attached are new patches, I simplified the use of free in the fixes of
pg_dumpall.
-- 
Michael
From d16f5965ca1eecf70a734d0bf30bd68dcbb65613 Mon Sep 17 00:00:00 2001
From: Michael Paquier 
Date: Mon, 8 Jun 2015 14:52:28 +0900
Subject: [PATCH 1/2] Fix some memory leaks in pg_dump and pg_dumpall

---
 src/bin/pg_dump/pg_dump.c| 83 
 src/bin/pg_dump/pg_dumpall.c |  4 +++
 2 files changed, 57 insertions(+), 30 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a72dfe9..7458389 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -225,7 +225,7 @@ static char *format_function_signature(Archive *fout,
 static char *convertRegProcReference(Archive *fout,
 		const char *proc);
 static char *convertOperatorReference(Archive *fout, const char *opr);
-static const char *convertTSFunction(Archive *fout, Oid funcOid);
+static char *convertTSFunction(Archive *fout, Oid funcOid);
 static Oid	findLastBuiltinOid_V71(Archive *fout, const char *);
 static Oid	findLastBuiltinOid_V70(Archive *fout);
 static void selectSourceSchema(Archive *fout, const char *schemaName);
@@ -10454,10 +10454,12 @@ dumpFunc(Archive *fout, DumpOptions *dopt, FuncInfo *finfo)
 		parseOidArray(protrftypes, typeids, FUNC_MAX_ARGS);
 		for (i = 0; typeids[i]; i++)
 		{
+			char	   *elemType;
 			if (i != 0)
 appendPQExpBufferStr(q, ", ");
-			appendPQExpBuffer(q, "FOR TYPE %s",
-		 getFormattedTypeName(fout, typeids[i], zeroAsNone));
+			elemType = getFormattedTypeName(fout, typeids[i], zeroAsNone);
+			appendPQExpBuffer(q, "FOR TYPE %s", elemType);
+			free(elemType);
 		}
 	}
 
@@ -10593,6 +10595,8 @@ dumpCast(Archive *fout, DumpOptions *dopt, CastInfo *cast)
 	PQExpBuffer delqry;
 	PQExpBuffer labelq;
 	FuncInfo   *funcInfo = NULL;
+	char	   *sourceType;
+	char	   *targetType;
 
 	/* Skip if not to be dumped */
 	if (!cast->dobj.dump || dopt->dataOnly)
@@ -10616,13 +10620,13 @@ dumpCast(Archive *fout, DumpOptions *dopt, CastInfo *cast)
 	delqry = createPQExpBuffer();
 	labelq = createPQExpBuffer();
 
+	sourceType = getFormattedTypeName(fout, cast->castsource, zeroAsNone);
+	targetType = getFormattedTypeName(fout, cast->casttarget, zeroAsNone);
 	appendPQExpBuffer(delqry, "DROP CAST (%s AS %s);\n",
-	getFormattedTypeName(fout, cast->castsource, zeroAsNone),
-   getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
+	  sourceType, targetType);
 
 	appendPQExpBuffer(defqry, "CREATE CAST (%s AS %s) ",
-	getFormattedTypeName(fout, cast->castsource, zeroAsNone),
-   getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
+	  sourceType, targetType);
 
 	switch (cast->castmethod)
 	{
@@ -10660,8 +10664,7 @@ dumpCast(Archive *fout, DumpOptions *dopt, CastInfo *cast)
 	appendPQExpBufferStr(defqry, ";\n");
 
 	appendPQExpBuffer(labelq, "CAST (%s AS %s)",
-	getFormattedTypeName(fout, cast->castsource, zeroAsNone),
-   getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
+	  sourceType, targetType);
 
 	if (dopt->binary_upgrade)
 		binary_upgrade_extension_member(defqry, &cast->dobj, labelq->data);
@@ -10679,6 +10682,9 @@ dumpCast(Archive *fout, DumpOptions *dopt, CastInfo *cast)
 NULL, "",
 cast->dobj.catId, 0, cast->dobj.dumpId);
 
+	free(sourceType);
+	free(targetType);
+
 	destroyPQExpBuffer(defqry);
 	destroyPQExpBuffer(delqry);
 	destroyPQExpBuffer(labelq);
@@ -10696,6 +10702,7 @@ dumpTransform(Archive *fout, DumpOptions *dopt, TransformInfo *transform)
 	FuncInfo   *fromsqlFuncInfo = NULL;
 	FuncInfo   *tosqlFuncInfo = NULL;
 	char	   *lanname;
+	char	   *transformType;
 
 	/* Skip if not to be dumped */
 	if (!transform->dobj.dump || dopt->dataOnly)
@@ -10723,13 +10730,14 @@ dumpTransform(Archive *fout, DumpOptions *dopt, TransformInfo *transform)
 	labelq = createPQExpBuffer();
 
 	lanname = get_language_name(fout, transform->trflang);
+	transformType = getFormattedTypeName(fout, transform->trftype, zeroAsNone);
 
 	appendPQExpBuffer(delqry, "DROP TRANSFORM FOR %s LANGUAGE %s;\n",
-  getFormattedTypeName(fout, transform->trftype, zeroAsNone),
+	  tran

Re: [HACKERS] [CORE] Restore-reliability mode

[Geoff Winkless]

Among several others, On 8 June 2015 at 13:59, David Gould 
wrote:

> I think Alphas are valuable and useful and even more so if they have
> release
> notes. For example, some of my clients are capable of fetching sources and
> building from scratch and filing bug reports and are often interested in
> particular new features. They even have staging infrastructure that could
> test new postgres releases with real applications. But they don't do it.
> They also don't follow -hackers, they don't track git, and they don't have
> any easy way to tell if if the new feature they are interested in is
> actually complete and ready to test at any particular time. A lot of
> features are developed in multiple commits over a period of time and they
> see no point in testing until at least most of the feature is complete and
> expected to work. But it is not obvious from outside when that happens for
> any given feature. For my clients the value of Alpha releases would
> mainly be the release notes, or some other mark in the sand that says "As
> of
> Alpha-3 feature X is included and expected to mostly work."
>

Wow! I never knew there were all these people out there who would be
rushing to help test if only the PG developers released alpha versions.
It's funny how they never used to do it when those alphas were done.

I say again: in my experience you don't get useful test reports from people
who aren't able or prepared to compile software; what you do get is lots of
unrelated and/or unhelpful noise in the mailing list. That may be harsh or
unfair or whatever, it's just my experience.

I guess the only thing we can do is see who's right. I'm simply trying to
point out that it's not the zero-cost exercise that everyone appears to
think that it is.

Geoff

Re: [HACKERS] [GENERAL] 9.4.1 -> 9.4.2 problem: could not access status of transaction 1

[Andres Freund]

On 2015-06-05 16:56:18 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > On June 5, 2015 10:02:37 PM GMT+02:00, Robert Haas  
> > wrote:
> >> I think we would be foolish to rush that part into the tree.  We
> >> probably got here in the first place by rushing the last round of
> >> fixes too much; let's try not to double down on that mistake.
>
> > My problem with that approach is that I think the code has gotten 
> > significantly more complex in the least few weeks. I have very little trust 
> > that the interactions between vacuum, the deferred truncations in the 
> > checkpointer, the state management in shared memory and recovery are 
> > correct. There's just too many non-local subtleties here.
>
> > I don't know what the right thing to do here is.
>
> My gut feeling is that rushing to make a release date is the wrong thing.
>
> If we have confidence that we can ship something on Monday that is
> materially more trustworthy than the current releases, then let's aim to
> do that; but let's ship only patches we are confident in.  We can do
> another set of releases later that incorporate additional fixes.  (As some
> wise man once said, there's always another bug.)

I've tortured hardware a fair bit with HEAD. So far it looks much better
than 9.4.2+ et al. I've noticed a bunch of, to me at least, new issues:

1) the autovacuum trigger logic isn't perfect yet. I.e. especially with
  autovacuum=off you can get into situations where emergency vacuums
  aren't started when necessary. This is particularly likely to happen
  if either very large multixacts are used, or if the server has been
  shut down while emergency autovacuum where happening. No corruption
  ensues, but it's not easy to get out of.

2) I've managed to corrupt a cluster when a standby performed
  restartpoints less frequently than the master performed
  checkpoints. Because truncations happen in the checkpointer it's not
  that hard to end up with entirely full multixact slrus. This is a
  problem on several fronts. We can IIUC end up truncating away the
  wrong data, and we can be in a bad state upon promotion.  None of that
  is new.

3) It's really confusing that truncation (and thus the limits in shared
  memory) happens in checkpoints. If you hit a limit and manually do all
  the necessary vacuums you'll see a "good" limit in
  pg_database.datminmxid, but you'll still into the error. You manually
  have to force a checkpoint for the truncation to actually
  happen. That's particularly problematic because larger installations,
  where I presume wraparound issues are more likely, often have a large
  checkpoint_timeout setting.

Since none of these are really new, I don't think they should prevent us
from doing a back branch release. While I'm still not convinced we're
better of with 9.4.4 than with 9.4.1, we're certainly better of than
with 9.4.[23] et al.

If we want to go ahead with the release I plan to do a bit more testing
today and tomorrow. If not I'm first going to continue working on fixing
the above.

I've a "good" fix for 1). I'm not 100% sure I'll feel confident with
pushing if we wrap today. I am wondering if we shouldn't at least apply
the portion that unconditionally sends a signal in the ERROR
case. That's still an improvement.


One more thing:
Our testing infrastructure sucks. Without writing C code it's basically
impossible to test wraparounds and such. Even if not particularly useful
for non-devs, I really think we should have functions for creating
burning xids/multixacts in core. Or at least in some extension.


-- 
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: Map basebackup tablespaces using a tablespace_map file

[Andrew Dunstan]

On 06/08/2015 12:08 AM, Amit Kapila wrote:
On Mon, Jun 8, 2015 at 5:52 AM, Andrew Dunstan > wrote:

>
> On 06/05/2015 11:08 PM, Amit Kapila wrote:
>>
>>
>> Okay, I think I can understand why you want to be cautious for
>> having a different check for this path, but in that case there is a
>> chance that recovery might fail when it will try to create a 
symlink
>> for that file.  Shouldn't we then try to call this new function 
only

>> when we are trying to restore from tablespace_map file and also
>> is there a need of ifdef S_ISLINK in remove_tablespace_link?
>>
>>
>> Shall I send an updated patch on these lines or do you want to
>> proceed with v3 version?
>>
>>
>
> The point seems to me that we should not be removing anything that's 
not an empty directory or symlink, and that nothing else has any 
business being in pg_tblspc. If we encounter such a thing whose name 
conflicts with the name of a tablespace link we wish to create, rather 
than quietly blowing it away we should complain loudly, and error out, 
making it the user's responsibility to clean up their mess. Am I 
missing something?

>

How about if it is just a flat file with same name as tablespace link,
why we want to give error for that case?  I think now it just don't do
anything with that file (unlink will fail with ENOENT and it will be
ignored, atleast thats the way currently it behaves in Windows) and
create a separate symlink with same name which seems okay to
me and in the change proposed by you it will give error, do you see
any reason for doing so?





This is surely wrong. unlink won't fail with ENOENT if the file is 
present; ENOENT means that the file is NOT present. It will succeed if 
the file is present, which is exactly what I'm saying is wrong.


I realize our existing code just more or less assumes that that it's a 
symlink. I think we've probably been a bit careless there.


cheers

andrew


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

[HACKERS] back-branch multixact fixes & 9.5 alpha/beta: schedule

[Robert Haas]

Hi,

I think we have consensus that we should proceed with releasing fixes
for the known multixact bugs in two stages:

- One set of minor releases with the fixes that we have now, to undo
the damage caused by 9.4.2 and still present in 9.4.3.  These changes
will force immediate anti-wraparound vacuums for some users in order
to repair bogus relminmxid, datminmxid, and control-file
oldestMultiXid values.  They will also fix failure-to-start problems
confirmed to exist in 9.4.2 and suspected problems with crash recovery
and recovery of an online backup.

- Another set of minor releases with the changes that Andres is
working on to add WAL-logging for multixact truncation.  I suppose
this will require the usual dance of upgrading the standby first and
then the master afterwards; there are details I'm not clear on yet
here.  This will fix other problems with recovery that are not new in
9.4.2 but go all the way back to 9.3.0.

In addition, it seems abundantly clear that everyone is very eager to
get some sort of 9.5 version out the door - if not a beta, then an
alpha.  I am still a bit concerned that's premature, but, on the other
hand, time is passing and at least some issues are getting dealt with
in the meantime, so... that's something.

So, when shall we do all of this releasing?  It seems like we could do
stage-one of the multixact fixing this week, and then figure out how
to do the other stuff after PGCon.  Alternatively, we can let the
latest round of changes that are already in the tree settle until
after PGCon, and plan a release for stage-one of the multixact fixing
then.  Whichever we pick, we then need to figure out the timetable for
the rest of it.

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

Re: [HACKERS] [CORE] Restore-reliability mode

[David Gould]

I think Alphas are valuable and useful and even more so if they have release
notes. For example, some of my clients are capable of fetching sources and
building from scratch and filing bug reports and are often interested in
particular new features. They even have staging infrastructure that could
test new postgres releases with real applications. But they don't do it.
They also don't follow -hackers, they don't track git, and they don't have
any easy way to tell if if the new feature they are interested in is
actually complete and ready to test at any particular time. A lot of
features are developed in multiple commits over a period of time and they
see no point in testing until at least most of the feature is complete and
expected to work. But it is not obvious from outside when that happens for
any given feature. For my clients the value of Alpha releases would
mainly be the release notes, or some other mark in the sand that says "As of
Alpha-3 feature X is included and expected to mostly work."

-dg

-- 
David Gould   da...@sonic.net
If simplicity worked, the world would be overrun with insects.


-- 
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] Collection of memory leaks for ECPG driver

[Michael Meskes]

On Mon, Jun 08, 2015 at 01:57:32PM +0900, Michael Paquier wrote:
> Please find attached a patch aimed at fixing a couple of memory leaks
> in the ECPG driver. Coverity (and sometimes I after reading some other
> code paths) found them.

And some are not correct it seems. But then some of the code isn't either. :)

> diff --git a/src/interfaces/ecpg/compatlib/informix.c 
> b/src/interfaces/ecpg/compatlib/informix.c
> index d6de3ea..c1e3dfb 100644
> --- a/src/interfaces/ecpg/compatlib/informix.c
> +++ b/src/interfaces/ecpg/compatlib/informix.c
> @@ -672,6 +672,7 @@ intoasc(interval * i, char *str)
>   if (!str)
>   return -errno;
>  
> + free(str);
>   return 0;
>  }

This function never worked it seems, we need to use a temp string, copy it to 
str and then free the temp one.

> diff --git a/src/interfaces/ecpg/ecpglib/connect.c 
> b/src/interfaces/ecpg/ecpglib/connect.c
> index 55c5680..12f445d 100644
> --- a/src/interfaces/ecpg/ecpglib/connect.c
> +++ b/src/interfaces/ecpg/ecpglib/connect.c
> @@ -298,7 +298,8 @@ ECPGconnect(int lineno, int c, const char *name, const 
> char *user, const char *p
>   envname = getenv("PG_DBPATH");
>   if (envname)
>   {
> - ecpg_free(dbname);
> + if (dbname)
> + ecpg_free(dbname);
>   dbname = ecpg_strdup(envname, lineno);
>   }

This is superfluous, at least with glibc. free()'s manpage clearly says "If
ptr is NULL, no operation is performed.", so why should we check again? Or do
we have architectures where free(NULL) is not a noop?

> diff --git a/src/interfaces/ecpg/preproc/descriptor.c 
> b/src/interfaces/ecpg/preproc/descriptor.c
> index 053a7af..ebd95d3 100644
> --- a/src/interfaces/ecpg/preproc/descriptor.c
> +++ b/src/interfaces/ecpg/preproc/descriptor.c

Do we really have to worry about these in a short running application like a 
precompiler, particularly if they add more than a simple free() command?

But then, you already did the work, so why not. Anyway, please tell me if you 
want to update the patch or if I should commit whatever is clear already.

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL


-- 
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] CREATE POLICY and RETURNING

[Dean Rasheed]

On 6 June 2015 at 22:16, Peter Geoghegan  wrote:
> On Fri, Oct 17, 2014 at 5:34 AM, Andres Freund  wrote:
>> On 2014-10-17 14:57:03 +0800, Craig Ringer wrote:
>>> On 10/17/2014 02:49 AM, Robert Haas wrote:
>>> > I think you could probably make the DELETE policy control what can get
>>> > deleted, but then have the SELECT policy further filter what gets
>>> > returned.
>>>
>>> That seems like the worst of both worlds to me.
>>>
>>> Suddenly DELETE ... RETURNING might delete more rows than it reports a
>>> resultset for. As well as being potentially dangerous for people using
>>> it in wCTEs, etc, to me that's the most astonishing possible outcome of all.
>>>
>>> I'd be much happier with even:
>>>
>>>   ERROR: RETURNING not permitted with SELECT row-security policy
>>
>> FWIW, that doesn't sound acceptable to me.
>
> This is more or less what ended up happening with UPSERT and USING
> security barrier quals on UPDATE/ALL policies. Realistically, the
> large majority of use cases don't involve a user being able to
> INSERT/DELETE tuples, but not SELECT them, and those that do should
> not be surprised to have a RETURNING fail (it's an odd enough union of
> different features that this seems acceptable to me).
>
> Like Fujii, I think that RETURNING with RLS should not get to avoid
> SELECT policies. I agree with the concern about not seeing affected
> rows with a DELETE (which, as I said, is very similar to UPSERT +
> WCO_RLS_CONFLICT_CHECK policies), so an error seems like the only
> alternative.
>
> The argument against not requiring SELECT *column* privilege on the
> EXCLUDED.* pseudo relation for UPSERT might have been: "well, what can
> be the harm of allowing the user to see what they themselves might
> have inserted?". But that would have been a bad argument then had
> anyone made it, because RETURNING with a (vanilla) INSERT requires
> SELECT privilege, and that's also what the user then actually inserted
> (as distinct from what the user *would have* inserted had the insert
> path been taking, representing as the EXCLUDED.* pseudo relation --
> for security purposes, ISTM that this is really no distinction at
> all). Consider before row insert triggers that can modify EXCLUDED.*
> tuples in a privileged way.
>
> So, the only logical reason that INSERT with RETURNING requires SELECT
> column privilege that I can see is that a before row INSERT trigger
> could modify the tuple inserted in a way that the inserter role should
> not know the details of. This long standing convention was reason
> enough to mandate that SELECT column privilege be required for the
> EXCLUDED.* pseudo relation for UPSERT. And so, I think it isn't too
> much of a jump to also say that we should do the same for RLS (for
> INSERTs for the reason I state, but also for UPDATEs and DELETEs for a
> far more obvious reason: the *existing* tuple can be projected, and
> the updater/deleter might well have no business seeing its contents).
>
> In short: I think we should be tracking a new WCOKind (perhaps
> WCO_RLS_RETURNING_CHECK?), that independently holds the security
> barrier quals as WCO-style checks when that's recognized as being
> necessary. For INSERT, these WCOs must be enforced against the target
> tuple projected by RETURNING. For UPDATEs and DELETEs, FROM/USING
> relations must also have SELECT privilege enforced against the
> projected target tuple, as well as the non-target relation --
> apparently the latter isn't currently happening, although Dean has
> tried to address this with his recent patch [1]. That is, even
> non-target relations (UPDATE ... FROM relations, or DELETE ... USING
> relations) do not have SELECT policy enforcement, but rather have
> UPDATE or DELETE policy enforcement only. I must admit that I was
> rather surprised at that; it has to be a bug.
>

Yes, I think a query with a RETURNING clause ought to either succeed
and return everything the user asked for, or error out if some/all of
the data isn't visible to the user according to SELECT policies in
effect. I think not applying the SELECT policies is wrong, and
returning a portion of the data updated would just be confusing.

My previous patch causes the SELECT policies for all the non-target
relations to be applied, but not the SELECT policies for the target
relation. So I think it would suffice to just add another check to
apply them to the target tuple, using something like
WCO_RLS_RETURNING_CHECK, as you suggested. However, I think it must be
applied to the target tuple *before* projecting it because the
RETURNING expressions might contain malicious leaky functions.
Actually I'm not sure the policy can be enforced against the projected
target tuple, since it might not contain all the necessary columns to
do the check.

In principle, it sounds easy to do, but I think it will be much
simpler against my previous patch.

Regards,
Dean


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

Re: [HACKERS] RLS fails to work with UPDATE ... WHERE CURRENT OF

[Dean Rasheed]

On 6 June 2015 at 23:28, Peter Geoghegan  wrote:
> Attached test case patch shows how RLS fails to play nice with UPDATE
> ... WHERE CURRENT OF.
> [snip]
> What's actually occurring here is that the executor imagines that this
> involves a foreign table scan (although I suppose it's equivocating a
> little bit by not saying so explicitly) -- ExecEvalCurrentOfExpr()
> comments imply that that's the only reason why control should reach it
> in practice. It looks like RLS has added a new way that CURRENT OF can
> fail to be made into a TidScan qualification. It doesn't look like
> Dean's most recent round of RLS fixes [1] addressed this case, based
> on his remarks. This non-support of WHERE CURRENT OF certainly isn't
> documented, and so looks like a bug.
>

Well spotted. I agree that this is a bug. We could just say that WHERE
CURRENT OF isn't supported on a table with RLS, but I think that would
be overly limiting.


> Unfortunately, the fact that WHERE CURRENT OF doesn't already accept
> additional qualifications doesn't leave me optimistic about this bug
> being easy to fix -- consider the gymnastics performed by commit
> c29a9c37 to get an idea of what I mean. Maybe it should just be
> formally desupported with RLS, as a stopgap solution for 9.5.
>
> [1] 
> http://www.postgresql.org/message-id/caezatcve7hdtfzgcjn-oevvawbtbgg8-fbch9vhdbhuzrsw...@mail.gmail.com

Actually I think it is fixable just by allowing the CURRENT OF
expression to be pushed down into the subquery through the security
barrier view. The planner is then guaranteed to generate a TID scan,
filtering by any other RLS quals, which ought to be the optimal plan.
Patch attached.

Regards,
Dean
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
new file mode 100644
index 0b83189..888eeac
*** a/src/backend/optimizer/path/allpaths.c
--- b/src/backend/optimizer/path/allpaths.c
*** subquery_push_qual(Query *subquery, Rang
*** 2177,2182 
--- 2177, 
  		recurse_push_qual(subquery->setOperations, subquery,
  		  rte, rti, qual);
  	}
+ 	else if (IsA(qual, CurrentOfExpr))
+ 	{
+ 		/*
+ 		 * This is possible when a WHERE CURRENT OF expression is applied to a
+ 		 * table with row-level security.  In that case, the subquery should
+ 		 * contain precisely one rtable entry for the table, and we can safely
+ 		 * push the expression down into the subquery.  This will cause a TID
+ 		 * scan subquery plan to be generated allowing the target relation to
+ 		 * be updated.
+ 		 *
+ 		 * Someday we might also be able to use a WHERE CURRENT OF expression
+ 		 * on a view, but currently the rewriter prevents that, so we should
+ 		 * never see any other case here, but generate sane error messages in
+ 		 * case it does somehow happen.
+ 		 */
+ 		if (subquery->rtable == NIL)
+ 			ereport(ERROR,
+ 	(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ 	 errmsg("WHERE CURRENT OF is not supported on a view with no underlying relation")));
+ 
+ 		if (list_length(subquery->rtable) > 1)
+ 			ereport(ERROR,
+ 	(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ 	 errmsg("WHERE CURRENT OF is not supported on a view with more than one underlying relation")));
+ 
+ 		if (subquery->hasAggs || subquery->groupClause || subquery->groupingSets || subquery->havingQual)
+ 			ereport(ERROR,
+ 	(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ 	 errmsg("WHERE CURRENT OF is not supported on a view with grouping or aggregation")));
+ 
+ 		/*
+ 		 * Adjust the CURRENT OF expression to refer to the underlying table
+ 		 * in the subquery, and attach it to the subquery's WHERE clause.
+ 		 */
+ 		qual = copyObject(qual);
+ 		((CurrentOfExpr *) qual)->cvarno = 1;
+ 
+ 		subquery->jointree->quals =
+ 			make_and_qual(subquery->jointree->quals, qual);
+ 	}
  	else
  	{
  		/*
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
new file mode 100644
index d40083d..0137e0e
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
*** contain_leaked_vars_walker(Node *node, v
*** 1492,1497 
--- 1492,1507 
  			}
  			break;
  
+ 		case T_CurrentOfExpr:
+ 
+ 			/*
+ 			 * WHERE CURRENT OF doesn't contain function calls.  Moreover, it
+ 			 * is important that this can be pushed down into a
+ 			 * security_barrier view, since the planner must always generate
+ 			 * a TID scan when CURRENT OF is present -- c.f. cost_tidscan.
+ 			 */
+ 			return false;
+ 
  		default:
  
  			/*
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
new file mode 100644
index 0ae5557..7c25349
*** a/src/test/regress/expected/rowsecurity.out
--- b/src/test/regress/expected/rowsecurity.out
*** COPY copy_t FROM STDIN; --fail - permiss
*** 2729,2734 
--- 2729,2841 
  ERROR:  permission denied for relation copy_t
  RESET SESSION AUTHORIZATION;
  DROP TABLE copy_t;
+ -- Check WHERE CURRENT OF
+ SET SESSION

Re: [HACKERS] pg_stat_archiver issue with aborted archiver

[Julien Rouhaud]

Le 08/06/2015 05:56, Michael Paquier a écrit :
> On Sun, Jun 7, 2015 at 1:11 AM, Julien Rouhaud 
>  wrote:
>> I just noticed that if the archiver aborts (for instance if the 
>> archive_command exited with a return code > 127),
>> pg_stat_archiver won't report those failed attempts. This happens
>> with both 9.4 and 9.5 branches.
>> 
>> Please find attached a patch that fix this issue, based on
>> current head.
> 
> The current code seems right to me. When the archive command dies 
> because of a signal (exit code > 128), the server should fail 
> immediately with FATAL and should not do any extra processing.

Ok. It may be worth to document it though.

> It will also try to archive again the same segment file after
> restart. When trying again, if this time the failure is not caused
> by a signal but still fails it will be reported to
> pg_stat_archiver.
> 

Yes, my comment was only about the failure not reported in some
special cases.

Thank for your response.
-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org


-- 
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] checkpointer continuous flushing

[Fabien COELHO]

Hello Cédric,


It looks a bit hazardous, do you have a benchmark for freeBSD ?


No, I just consulted the FreeBSD man page for posix_fadvise. I someone can 
run tests on something which HDDs is not linux, that would be nice.



Sources says:
case POSIX_FADV_DONTNEED:
/*
 * Flush any open FS buffers and then remove pages
 * from the backing VM object.  Using vinvalbuf() here
 * is a bit heavy-handed as it flushes all buffers for
 * the given vnode, not just the buffers covering the
 * requested range.


It is indeed heavy-handed, but that would probably trigger the expected 
behavior which is to start writing to disk, so I would expect to see 
benefits similar to those of "sync_file_range" on Linux.


Buffer writes from bgwriter & checkpointer are throttled, which reduces 
the potential impact of a "heavy-handed" approach in the kernel.


Now if on some platforms the behavior is absurd, obviously it would be 
better to turn the feature off on those.


Note that this is already used by pg in "initdb", but the impact would 
probably be very small anyway.


--
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] checkpointer continuous flushing

[Cédric Villemain]

Le 07/06/2015 16:53, Fabien COELHO a écrit :
> +» » /*·Others:·say·that·data·should·not·be·kept·in·memory...
> +» » ·*·This·is·not·exactly·what·we·want·to·say,·because·we·want·to·write
> +» » ·*·the·data·for·durability·but·we·may·need·it·later·nevertheless.
> +» » ·*·It·seems·that·Linux·would·free·the·memory·*if*·the·data·has
> +» » ·*·already·been·written·do·disk,·else·it·is·ignored.
> +» » ·*·For·FreeBSD·this·may·have·the·desired·effect·of·moving·the
> +» » ·*·data·to·the·io·layer.
> +» » ·*/
> +» » rc·=·posix_fadvise(context->fd,·context->offset,·context->nbytes,
> +» » » » » » ···POSIX_FADV_DONTNEED);
> +

It looks a bit hazardous, do you have a benchmark for freeBSD ?

Sources says:
case POSIX_FADV_DONTNEED:
/*
 * Flush any open FS buffers and then remove pages
 * from the backing VM object.  Using vinvalbuf() here
 * is a bit heavy-handed as it flushes all buffers for
 * the given vnode, not just the buffers covering the
 * requested range.

-- 
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation


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