Re: [HACKERS] Confusing recovery message when target not hit

2016-06-12 Thread Michael Paquier
On Mon, Jun 13, 2016 at 9:53 AM, Thom Brown  wrote:
> On 12 June 2016 at 12:51, Michael Paquier  wrote:
>>
>> On Sun, Jun 12, 2016 at 7:52 PM, Thom Brown  wrote:
>> > Aren't those already set by recoveryStopsBefore()?
>>
>> It is possible to exit the main redo loop if a NULL record is found
>> after calling ReadRecord, in which case those would not be set, no?
>
>
> I'm apprehensive about initialising those values myself as I don't want to
> set them at a point where they may potentially already be set.

As your patch relies on checks on the variables holding the recovery
stop information as not being set, initializing them before entering
in the REDO phase (say just before 6435:xlog.c on HEAD) is the safest
thing to do IMO.
-- 
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] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-12 Thread Tatsuro Yamada

Hi,


Subject: Re: [HACKERS] ORDER/GROUP BY expression not found in targetlist
Date: Thu, 09 Jun 2016 12:08:01 +0900


Right, I saw that thread which involved the same error message:

https://www.postgresql.org/message-id/flat/20160526021235.w4nq7k3gnheg7vit%40alap3.anarazel.de#20160526021235.w4nq7k3gnheg7...@alap3.anarazel.de

... but that seems to be a different problem than the one you and I
have seen, it involved repeated references to columns in the tlist.
It was fixed with this commit:

commit aeb9ae6457865c8949641d71a9523374d843a418
Author: Tom Lane 
Date:   Thu May 26 14:52:24 2016 -0400

 Disable physical tlist if any Var would need multiple sortgroupref labels.


I use this version:f721e94 to run tpc-h on last week.
This patch is commited at Jun 8. If it fixed, I didn't get the error.

>PG96beta1
>  commit: f721e94b5f360391fc3ffe183bf697a0441e9184

-
commit f721e94b5f360391fc3ffe183bf697a0441e9184
Author: Robert Haas 
Date:   Wed Jun 8 08:37:06 2016 -0400

Fix typo.

Amit Langote
-

I got mistake to write an e-mail to -hackers on last week. :-<
I should have written this.

  The bug has not fixed by Tom Lane's patch: commit aeb9ae6.
  Because I got the same error using tpc-h.



Today, I try it again by changing max_parallel_workers_per_gather parameter.
The result of Q1 is bellow. Is this bug in the Open items on wiki?


I don't see it on the Open Issues list.


I checked the list, but the bug is not listed.
  https://wiki.postgresql.org/wiki/PostgreSQL_9.6_Open_Items


Regards,
Tatsuro Yamada
NTT OSS Center






--
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] Reviewing freeze map code

2016-06-12 Thread Amit Kapila
On Sat, Jun 11, 2016 at 1:24 AM, Robert Haas  wrote:
>
> 3. vacuumlazy.c includes this code:
>
> if (heap_prepare_freeze_tuple(tuple.t_data, FreezeLimit,
>   MultiXactCutoff,
[nfrozen]))
> frozen[nfrozen++].offset = offnum;
> else if (heap_tuple_needs_eventual_freeze(tuple.t_data))
> all_frozen = false;
>
> That's wrong, because a "true" return value from
> heap_prepare_freeze_tuple() means only that it has done *some*
> freezing work on the tuple, not that it's done all of the freezing
> work that will ever need to be done.  So, if the tuple's xmin can be
> frozen and is aborted but not older than vacuum_freeze_min_age, then
> heap_prepare_freeze_tuple() won't free xmax, but the page will still
> be marked all-frozen, which is bad.
>

To clarify, are you talking about a case where insertion has aborted?
Won't in such a case all_visible flag be set to false due to return value
from HeapTupleSatisfiesVacuum() and if so, later code shouldn't mark it as
all_frozen?


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


Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-12 Thread David Rowley
On 13 June 2016 at 15:39, Thomas Munro  wrote:
> What is going on here?

...

>
> postgres=# set max_parallel_workers_per_gather = 2;
> SET
> postgres=# explain select length(data) from logs group by length(data);
> ERROR:  ORDER/GROUP BY expression not found in targetlist

Seems like this was caused by 04ae11f62e643e07c411c4935ea6af46cb112aa9

I missed the discussion on this commit, so I'll go look for that now.

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


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


Re: [HACKERS] WIP: Data at rest encryption

2016-06-12 Thread Michael Paquier
On Sun, Jun 12, 2016 at 4:13 PM, Ants Aasma  wrote:
>> I feel separate file is better to include the key data instead of pg_control
>> file.
>
> I guess that would be more flexible. However I think at least the fact
> that the database is encrypted should remain in the control file to
> provide useful error messages for faulty backup procedures.

Another possibility could be always to do some encryption at data-type
level for text data. For example I recalled the following thing while
going through this thread:
https://github.com/nec-postgres/tdeforpg
Though I don't quite understand the use for encrypt.enable in this
code... This has the advantage to not patch upstream.
-- 
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] WIP: Data at rest encryption

2016-06-12 Thread Ants Aasma
On Fri, Jun 10, 2016 at 5:23 AM, Haribabu Kommi
 wrote:
> 1. Instead of doing the entire database files encryption, how about
> providing user an option to protect only some particular tables that
> wants the encryption at table/tablespace level. This not only provides
> an option to the user, it reduces the performance impact on tables
> that doesn't need any encryption. The problem with this approach
> is that every xlog record needs to validate to handle the encryption
> /decryption, instead of at page level.

Is there a real need for this? The customers I have talked to want to
encrypt the whole database and my goal is to make the feature fast
enough to make that feasible for pretty much everyone. I guess
switching encryption off per table would be feasible, but the key
setup would still need to be done at server startup. Per record
encryption would result in some additional information leakage though.
Overall I thought it would not be worth it, but I'm willing to have my
mind changed on this.

> 2. Instead of depending on a contrib module for the encryption, how
> about integrating pgcrypto contrib in to the core and add that as a
> default encryption method. And also provide an option to the user
> to use a different encryption methods if needs.

Technically that would be simple enough, this is more of a policy
decision. I think having builtin encryption provided by pgcrypto is
completely fine. If a consensus emerges that it needs to be
integrated, it would need to be a separate patch anyway.

> 3. Currently entire xlog pages are encrypted and stored in the file.
> can pg_xlogdump works with those files?

Technically yes, with the patch as it stands, no. Added this to my todo list.

> 4. For logical decoding, how about the adding a decoding behavior
> based on the module to decide whether data to be encrypted/decrypted.

The data to be encrypted does not depend on the module used, so I
don't think it should be module controlled. The reorder buffer
contains pretty much the same stuff as the xlog, so not encrypting it
does not look like a valid choice. For logical heap rewrites it could
be argued that nothing useful is leaked in most cases, but encrypting
it is not hard. Just a small matter of programming.

> 5. Instead of providing passphrase through environmental variable,
> better to provide some options to pg_ctl etc.

That looks like it would be worse from a security perspective.
Integrating a passphrase prompt would be an option, but a way for
scripts to provide passphrases would still be needed.

> 6. I don't have any idea whether is it possible to integrate the checksum
> and encryption in a single shot to avoid performance penalty.

Currently no, the checksum gets stored in the page header and for any
decent cipher mode the encryption of the rest of the page will depend
on it. However, the performance difference should be negligible
because both algorithms are compute bound for cached data. The data is
very likely to be completely in L1 cache as the operations are done in
quick succession.

The non-cryptographic checksum algorithm could actually be an attack
vector for an adversary that can trigger repeated encryption by
tweaking a couple of bytes at the end of the page to see when the
checksum matches and try to infer the data from that. Similarly to the
CRIME attack. However the LSN stored at the beginning of the page
header basically provides a nonce that makes this impossible.

This also means that encryption needs to imply wal_log_hints. Will
include this in the next version of the patch.

>> I would also like to incorporate some database identifier as a salt in
>> key setup. However, system identifier stored in control file doesn't
>> fit this role well. It gets initialized somewhat too late in the
>> bootstrap process, and more importantly, gets changed on pg_upgrade.
>> This will make link mode upgrades impossible, which seems like a no
>> go. I'm torn whether to add a new value for this purpose (perhaps
>> stored outside the control file) or allow setting of system identifier
>> via initdb. The first seems like a better idea, the file could double
>> as a place to store additional encryption parameters, like key length
>> or different cipher primitive.
>
> I feel separate file is better to include the key data instead of pg_control
> file.

I guess that would be more flexible. However I think at least the fact
that the database is encrypted should remain in the control file to
provide useful error messages for faulty backup procedures.

Thanks for your input.

Regards,
Ants Aasma


-- 
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] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-12 Thread Thomas Munro
On Mon, Jun 13, 2016 at 4:16 PM, Tatsuro Yamada
 wrote:
> I tried to run tpc-h queries, but some queries failed by the error on last
> week.
>
>>Subject: Re: [HACKERS] ORDER/GROUP BY expression not found in targetlist
>>Date: Thu, 09 Jun 2016 12:08:01 +0900

Right, I saw that thread which involved the same error message:

https://www.postgresql.org/message-id/flat/20160526021235.w4nq7k3gnheg7vit%40alap3.anarazel.de#20160526021235.w4nq7k3gnheg7...@alap3.anarazel.de

... but that seems to be a different problem than the one you and I
have seen, it involved repeated references to columns in the tlist.
It was fixed with this commit:

commit aeb9ae6457865c8949641d71a9523374d843a418
Author: Tom Lane 
Date:   Thu May 26 14:52:24 2016 -0400

Disable physical tlist if any Var would need multiple sortgroupref labels.

> Today, I try it again by changing max_parallel_workers_per_gather parameter.
> The result of Q1 is bellow. Is this bug in the Open items on wiki?

I don't see it on the Open Issues list.

-- 
Thomas Munro
http://www.enterprisedb.com


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


Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-12 Thread Tatsuro Yamada

Hi,

I tried to run tpc-h queries, but some queries failed by the error on last week.

>Subject: Re: [HACKERS] ORDER/GROUP BY expression not found in targetlist
>Date: Thu, 09 Jun 2016 12:08:01 +0900

Today, I try it again by changing max_parallel_workers_per_gather parameter.
The result of Q1 is bellow. Is this bug in the Open items on wiki?

-
postgres=# set max_parallel_workers_per_gather = 0;
SET
postgres=# \i queries/1.explain.sql
 QUERY PLAN
-
 Limit  (cost=43474.03..43474.03 rows=1 width=236) (actual 
time=1039.583..1039.583 rows=1 loops=1)
   ->  Sort  (cost=43474.03..43474.04 rows=6 width=236) (actual 
time=1039.583..1039.583 rows=1 loops=1)
 Sort Key: l_returnflag, l_linestatus
 Sort Method: top-N heapsort  Memory: 25kB
 ->  HashAggregate  (cost=43473.83..43474.00 rows=6 width=236) (actual 
time=1039.529..1039.534 rows=4 loops=1)
   Group Key: l_returnflag, l_linestatus
   ->  Seq Scan on lineitem  (cost=0.00..19668.15 rows=595142 
width=25) (actual time=0.048..125.332 rows=595224 loops=1)
 Filter: (l_shipdate <= '1998-09-22 00:00:00'::timestamp 
without time zone)
 Rows Removed by Filter: 5348
 Planning time: 0.180 ms
 Execution time: 1039.758 ms
(11 rows)

postgres=# set max_parallel_workers_per_gather = default;
SET
postgres=# \i queries/1.explain.sql
ERROR:  ORDER/GROUP BY expression not found in targetlist

-

Regards,
Tatsuro Yamada
NTT OSS Center


On 2016/06/13 12:39, Thomas Munro wrote:

Hi,

What is going on here?

postgres=# create table logs as select generate_series(1,
100)::text as data;
SELECT 100
postgres=# insert into logs select * from logs;
INSERT 0 100
postgres=# insert into logs select * from logs;
INSERT 0 200
postgres=# insert into logs select * from logs;
INSERT 0 400
postgres=# insert into logs select * from logs;
INSERT 0 800
postgres=# insert into logs select * from logs;
INSERT 0 1600
postgres=# analyze logs;
ANALYZE
postgres=# set max_parallel_workers_per_gather = 0;
SET
postgres=# explain select length(data) from logs group by length(data);
┌┐
│ QUERY PLAN │
├┤
│ Group  (cost=5843157.07..6005642.13 rows=993989 width=4)   │
│   Group Key: (length(data))│
│   ->  Sort  (cost=5843157.07..5923157.11 rows=3218 width=4)│
│ Sort Key: (length(data))   │
│ ->  Seq Scan on logs  (cost=0.00..541593.22 rows=3218 width=4) │
└┘
(5 rows)

postgres=# set max_parallel_workers_per_gather = 2;
SET
postgres=# explain select length(data) from logs group by length(data);
ERROR:  ORDER/GROUP BY expression not found in targetlist






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


[HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-12 Thread Thomas Munro
Hi,

What is going on here?

postgres=# create table logs as select generate_series(1,
100)::text as data;
SELECT 100
postgres=# insert into logs select * from logs;
INSERT 0 100
postgres=# insert into logs select * from logs;
INSERT 0 200
postgres=# insert into logs select * from logs;
INSERT 0 400
postgres=# insert into logs select * from logs;
INSERT 0 800
postgres=# insert into logs select * from logs;
INSERT 0 1600
postgres=# analyze logs;
ANALYZE
postgres=# set max_parallel_workers_per_gather = 0;
SET
postgres=# explain select length(data) from logs group by length(data);
┌┐
│ QUERY PLAN │
├┤
│ Group  (cost=5843157.07..6005642.13 rows=993989 width=4)   │
│   Group Key: (length(data))│
│   ->  Sort  (cost=5843157.07..5923157.11 rows=3218 width=4)│
│ Sort Key: (length(data))   │
│ ->  Seq Scan on logs  (cost=0.00..541593.22 rows=3218 width=4) │
└┘
(5 rows)

postgres=# set max_parallel_workers_per_gather = 2;
SET
postgres=# explain select length(data) from logs group by length(data);
ERROR:  ORDER/GROUP BY expression not found in targetlist

-- 
Thomas Munro
http://www.enterprisedb.com

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


Re: [HACKERS] Why we don't have checksums on clog files

2016-06-12 Thread Tom Lane
Robert Haas  writes:
> On Tue, Jun 7, 2016 at 10:41 AM, Amit Kapila  wrote:
>> I think it will be better if users can have an option to checksum clog
>> pages.  However, I think that will need a change in page (CLOG-page) format
>> which might not be a trivial work to accomplish.

> Doesn't the pd_checksum field exist on SLRU pages also?

Uh, no.

(I don't think that's an inherent property of slru.c, but definitely
clog.c packs the pages totally fully of xact status bits.  See
CLOG_XACTS_PER_PAGE.)

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] Why we don't have checksums on clog files

2016-06-12 Thread Robert Haas
On Tue, Jun 7, 2016 at 10:41 AM, Amit Kapila  wrote:
> I think it will be better if users can have an option to checksum clog
> pages.  However, I think that will need a change in page (CLOG-page) format
> which might not be a trivial work to accomplish.

Doesn't the pd_checksum field exist on SLRU pages also?  If so, we
could just start using it.

-- 
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] increase message string buffer size of watch command of psql

2016-06-12 Thread Tom Lane
Robert Haas  writes:
> On Sun, Jun 12, 2016 at 10:55 AM, Ioseph Kim  wrote:
>> Increase size of this title, please.
>> 50 bytes is so small for multi language.
>> 
>> And. I suggest that date string might be local language,
>> or current_timestamp string.

> This was already changed in commit dea2b5960.

Well, we did part of that, but it's still using asctime().  Should we
change that to strftime(..."%c"...) to be less English-centric?
(The result seems to be the same in C locale.  pg_controldata has done
it that way for a long time, with few complaints.)  If we want to do so,
now would be the time, since 9.6 already whacked around the format
of \watch output.

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] increase message string buffer size of watch command of psql

2016-06-12 Thread Robert Haas
On Sun, Jun 12, 2016 at 10:55 AM, Ioseph Kim  wrote:
> Hello.
> In po.ko (korean message) at psql
> #: command.c:2971
> #, c-format
> msgid "Watch every %lds\t%s"
> msgstr "%ld초 간격으로 지켜보기\t%s"
>
> this message string is a cut string, because buffer size is small.
> At line 2946 in src/bin/psql/command.c
> chartitle[50];
>
> size of message string for korean is over 50 bytes.
> (at least 80 columns terminal for common)
>
> Increase size of this title, please.
> 50 bytes is so small for multi language.
>
> And. I suggest that date string might be local language,
> or current_timestamp string.

This was already changed in commit dea2b5960.

-- 
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] Rename max_parallel_degree?

2016-06-12 Thread Robert Haas
On Sat, Jun 11, 2016 at 6:24 PM, Julien Rouhaud
 wrote:
> On 11/06/2016 23:37, Julien Rouhaud wrote:
>> On 09/06/2016 16:04, Robert Haas wrote:
>>> OK, I pushed this after re-reviewing it and fixing a number of
>>> oversights.  There remains only the task of adding max_parallel_degree
>>> as a system-wide limit (as opposed to max_parallel_degree now
>>> max_parallel_workers_per_gather which is a per-Gather limit), which
>>> I'm going to argue should be a new open item and not necessarily one
>>> that I have to own myself.  I would like to take care of it, but I
>>> will not put it ahead of fixing actual defects and I will not promise
>>> to have it done in time for 9.6.
>>>
>>
>> PFA a patch to fix this open item.  I used the GUC name provided in the
>> 9.6 open item page (max_parallel_workers), with a default value of 4.
>> Value 0 is another way to disable parallel query.
>>
>
> Sorry I just realized I made a stupid mistake, I didn't check in all
> slots to get the number of active parallel workers. Fixed in attached v2.

I think instead of adding a "bool parallel" argument to
RegisterDynamicBackgroundWorker, we should just define a new constant
for bgw_flags, say BGW_IS_PARALLEL_WORKER.

-- 
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] Confusing recovery message when target not hit

2016-06-12 Thread Thom Brown
On 12 June 2016 at 12:51, Michael Paquier  wrote:

> On Sun, Jun 12, 2016 at 7:52 PM, Thom Brown  wrote:
> > Aren't those already set by recoveryStopsBefore()?
>
> It is possible to exit the main redo loop if a NULL record is found
> after calling ReadRecord, in which case those would not be set, no?
>

I'm apprehensive about initialising those values myself as I don't want to
set them at a point where they may potentially already be set.

And I've noticed that I didn't re-read my own output messages, so I've
corrected them in the attached patch.

Thom
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e4645a3..d724d4b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7105,19 +7105,34 @@ StartupXLOG(void)
 		 * timeline changed.
 		 */
 		if (recoveryTarget == RECOVERY_TARGET_XID)
-			snprintf(reason, sizeof(reason),
-	 "%s transaction %u",
-	 recoveryStopAfter ? "after" : "before",
-	 recoveryStopXid);
+			if (recoveryStopXid == InvalidTransactionId)
+snprintf(reason, sizeof(reason),
+		"recovery finished without reaching recovery target xid of \"%u\"\n",
+		recoveryTargetXid);
+			else
+snprintf(reason, sizeof(reason),
+		 "%s transaction %u",
+		 recoveryStopAfter ? "after" : "before",
+		 recoveryStopXid);
 		else if (recoveryTarget == RECOVERY_TARGET_TIME)
-			snprintf(reason, sizeof(reason),
-	 "%s %s\n",
-	 recoveryStopAfter ? "after" : "before",
-	 timestamptz_to_str(recoveryStopTime));
+			if (recoveryStopTime == 0)
+snprintf(reason, sizeof(reason),
+		"recovery finished without reaching recovery target time of \"%s\"\n",
+		timestamptz_to_str(recoveryTargetTime));
+			else
+snprintf(reason, sizeof(reason),
+		 "%s %s\n",
+		 recoveryStopAfter ? "after" : "before",
+		 timestamptz_to_str(recoveryStopTime));
 		else if (recoveryTarget == RECOVERY_TARGET_NAME)
-			snprintf(reason, sizeof(reason),
-	 "at restore point \"%s\"",
-	 recoveryStopName);
+			if (*recoveryStopName == '\0')
+snprintf(reason, sizeof(reason),
+		"recovery finished without reaching recovery target name of \"%s\"\n",
+		recoveryTargetName);
+			else
+snprintf(reason, sizeof(reason),
+		 "at restore point \"%s\"",
+		 recoveryStopName);
 		else if (recoveryTarget == RECOVERY_TARGET_IMMEDIATE)
 			snprintf(reason, sizeof(reason), "reached consistency");
 		else

-- 
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: [PERFORM] 9.4 -> 9.5 regression with queries through pgbouncer on RHEL 6

2016-06-12 Thread Andres Freund
Hi Vladimir,

Thanks for these reports.

On 2016-06-13 00:42:19 +0300, Vladimir Borodin wrote:
> perf report -g -i pg9?_all.data >/tmp/pg9?_perf_report.txt

Any chance you could redo the reports with --no-children --call-graph=fractal
added? The mode that includes child overheads unfortunately makes the
output hard to interpet/compare.

> The results from pg9?_perf_report.txt are attached. Note that in all cases 
> some events were lost, i.e.:
> 
> root@pgload05g ~ # perf report -g -i pg94_all.data >/tmp/pg94_perf_report.txt
> Failed to open [vsyscall], continuing without symbols
> Warning:
> Processed 537137 events and lost 7846 chunks!

You can reduce the overhead by reducing the sampling frequency, e.g. by
specifying -F 300.

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


[HACKERS] increase message string buffer size of watch command of psql

2016-06-12 Thread Ioseph Kim
Hello.
In po.ko (korean message) at psql 
#: command.c:2971
#, c-format
msgid "Watch every %lds\t%s"
msgstr "%ld초 간격으로 지켜보기\t%s"

this message string is a cut string, because buffer size is small.
At line 2946 in src/bin/psql/command.c 
chartitle[50];

size of message string for korean is over 50 bytes.
(at least 80 columns terminal for common)

Increase size of this title, please.
50 bytes is so small for multi language.

And. I suggest that date string might be local language,
or current_timestamp string.

Regards, Ioseph.



-- 
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] Confusing recovery message when target not hit

2016-06-12 Thread Michael Paquier
On Sun, Jun 12, 2016 at 7:52 PM, Thom Brown  wrote:
> Aren't those already set by recoveryStopsBefore()?

It is possible to exit the main redo loop if a NULL record is found
after calling ReadRecord, in which case those would not be set, no?
-- 
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] Confusing recovery message when target not hit

2016-06-12 Thread Thom Brown
On 11 June 2016 at 13:22, Michael Paquier  wrote:

> On Sat, Jun 11, 2016 at 9:44 AM, Thom Brown  wrote:
> > It may be the wrong way of going about it, but you get the idea of what
> I'm
> > suggesting we output instead.
>
> Surely things could be better. So +1 to be more verbose here.
>
> +if (recoveryStopTime == 0)
> +snprintf(reason, sizeof(reason),
> +"recovery reached consistency before recovery
> target time of \"%s\"\n",
> +timestamptz_to_str(recoveryTargetTime));
> "Reaching consistency" is not exact for here. I'd rather say "finished
> recovery without reaching target blah"
>

Yeah, sounds fine.


>
> +if (recoveryStopXid == 0)
> Checking for InvalidTransactionId is better here.
>

Agreed.


> And it would be good to initialize recoveryStopTime and
> recoveryStopXid as those are set only when a recovery target is
> reached.
>

Aren't those already set by recoveryStopsBefore()?

Revised patch attached, with new wording and covering recovery target name
case.

Thom
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e4645a3..4033a2d 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7105,19 +7105,34 @@ StartupXLOG(void)
 		 * timeline changed.
 		 */
 		if (recoveryTarget == RECOVERY_TARGET_XID)
-			snprintf(reason, sizeof(reason),
-	 "%s transaction %u",
-	 recoveryStopAfter ? "after" : "before",
-	 recoveryStopXid);
+			if (recoveryStopXid == InvalidTransactionId)
+snprintf(reason, sizeof(reason),
+		"recovery finished withoutrecovery target xid of \"%u\"\n",
+		recoveryTargetXid);
+			else
+snprintf(reason, sizeof(reason),
+		 "%s transaction %u",
+		 recoveryStopAfter ? "after" : "before",
+		 recoveryStopXid);
 		else if (recoveryTarget == RECOVERY_TARGET_TIME)
-			snprintf(reason, sizeof(reason),
-	 "%s %s\n",
-	 recoveryStopAfter ? "after" : "before",
-	 timestamptz_to_str(recoveryStopTime));
+			if (recoveryStopTime == 0)
+snprintf(reason, sizeof(reason),
+		"recovery finished without recovery target time of \"%s\"\n",
+		timestamptz_to_str(recoveryTargetTime));
+			else
+snprintf(reason, sizeof(reason),
+		 "%s %s\n",
+		 recoveryStopAfter ? "after" : "before",
+		 timestamptz_to_str(recoveryStopTime));
 		else if (recoveryTarget == RECOVERY_TARGET_NAME)
-			snprintf(reason, sizeof(reason),
-	 "at restore point \"%s\"",
-	 recoveryStopName);
+			if (*recoveryStopName == '\0')
+snprintf(reason, sizeof(reason),
+		"recovery finished without reaching recovery target name of \"%s\"\n",
+		recoveryTargetName);
+			else
+snprintf(reason, sizeof(reason),
+		 "at restore point \"%s\"",
+		 recoveryStopName);
 		else if (recoveryTarget == RECOVERY_TARGET_IMMEDIATE)
 			snprintf(reason, sizeof(reason), "reached consistency");
 		else

-- 
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] Confusing recovery message when target not hit

2016-06-12 Thread Michael Paquier
On Sun, Jun 12, 2016 at 12:46 AM, David Steele  wrote:
> On 6/11/16 8:22 AM, Michael Paquier wrote:
>> On Sat, Jun 11, 2016 at 9:44 AM, Thom Brown  wrote:
>>> It may be the wrong way of going about it, but you get the idea of what I'm
>>> suggesting we output instead.
>>
>> Surely things could be better. So +1 to be more verbose here.
>>
>> +if (recoveryStopTime == 0)
>> +snprintf(reason, sizeof(reason),
>> +"recovery reached consistency before recovery
>> target time of \"%s\"\n",
>> +timestamptz_to_str(recoveryTargetTime));
>> "Reaching consistency" is not exact for here. I'd rather say "finished
>> recovery without reaching target blah"
>>
>> +if (recoveryStopXid == 0)
>> Checking for InvalidTransactionId is better here.
>>
>> And it would be good to initialize recoveryStopTime and
>> recoveryStopXid as those are set only when a recovery target is
>> reached.
>
> +1 for Michael's wording.  It's not very clear in the logs right now if
> a recovery target was missed.  That makes it very difficult for the user
> to determine if PITR worked or not.

By the way, we surely want to have the same logic for a recovery
target name. It could be possible to reach the end of recovery without
reaching the goal of recovery.conf. It would be good to be verbose in
this case as well by checking for recoveryStopName[0] = '\0'.
-- 
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] parallel workers and client encoding

2016-06-12 Thread Noah Misch
On Thu, Jun 09, 2016 at 12:04:59PM -0400, Peter Eisentraut wrote:
> On 6/6/16 9:45 PM, Peter Eisentraut wrote:
> >There appears to be a problem with how client encoding is handled in the
> >communication from parallel workers.
> 
> I have added this to the open items for now.

[Action required within 72 hours.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] 
http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.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] Online DW

2016-06-12 Thread Craig Ringer
On 11 June 2016 at 12:29, Sridhar N Bamandlapally 
wrote:

> I need every transaction coming from application sync with both production
> and archive db,
>
> but the transactions I do to clean old data(before 7 days) on production
> db in daily maintenance window should not sync with archive db,
>
> Archive db need read-only, used for maintaining integrity with other
> business applications
>

In a separate mail to -general I've asked Sridhar to keep this discussion
to -general, not -hackers, from now on.


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