Re: [HACKERS] Confusing recovery message when target not hit
On Mon, Jun 13, 2016 at 9:53 AM, Thom Brownwrote: > 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
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 LaneDate: 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
On Sat, Jun 11, 2016 at 1:24 AM, Robert Haaswrote: > > 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
On 13 June 2016 at 15:39, Thomas Munrowrote: > 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
On Sun, Jun 12, 2016 at 4:13 PM, Ants Aasmawrote: >> 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
On Fri, Jun 10, 2016 at 5:23 AM, Haribabu Kommiwrote: > 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
On Mon, Jun 13, 2016 at 4:16 PM, Tatsuro Yamadawrote: > 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
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
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
Robert Haaswrites: > 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
On Tue, Jun 7, 2016 at 10:41 AM, Amit Kapilawrote: > 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
Robert Haaswrites: > 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
On Sun, Jun 12, 2016 at 10:55 AM, Ioseph Kimwrote: > 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?
On Sat, Jun 11, 2016 at 6:24 PM, Julien Rouhaudwrote: > 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
On 12 June 2016 at 12:51, Michael Paquierwrote: > 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
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
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
On Sun, Jun 12, 2016 at 7:52 PM, Thom Brownwrote: > 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
On 11 June 2016 at 13:22, Michael Paquierwrote: > 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
On Sun, Jun 12, 2016 at 12:46 AM, David Steelewrote: > 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
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
On 11 June 2016 at 12:29, Sridhar N Bamandlapallywrote: > 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