Re: Fix calculations on WAL recycling.
On Mon, Jul 23, 2018 at 01:57:48PM +0900, Kyotaro HORIGUCHI wrote: > I'll register this to the next commitfest. > > https://www.postgresql.org/message-id/20180719.125117.155470938.horiguchi.kyot...@lab.ntt.co.jp This is an open item for v11. >> While considering this, I found a bug in 4b0d28de06, which >> removed prior checkpoint from control file. It actually trims the >> segments before the last checkpoint's redo segment but recycling >> is still considered based on the *prevous* checkpoint. As the >> result min_wal_size doesn't work as told. Specifically, setting >> min/max_wal_size to 48MB and advance four or more segments then >> two checkpoints leaves just one segment, which is less than >> min_wal_size. >> >> The attached patch fixes that. One arguable point on this would >> be the removal of the behavior when RemoveXLogFile(name, >> InvalidXLogRecPtr, ..). >> >> The only place calling the function with the parameter is >> timeline switching. Previously unconditionally 10 segments are >> recycled after switchpoint but the reason for the behavior is we >> didn't have the information on previous checkpoint at hand at the >> time. But now we can use the timeline switch point as the >> approximate of the last checkpoint's redo point and this allows >> us to use min/max_wal_size properly at the time. I have been looking at that, and tested with this simple scenario: create table aa (a int); Then just repeat the following SQLs: insert into aa values (1); select pg_switch_wal(); checkpoint; select pg_walfile_name(pg_current_wal_lsn()); By doing so, you would notice that the oldest WAL segment does not get recycled after the checkpoint, while it should as the redo pointer is always checkpoint generated. What happens is that this oldest segment gets recycled every two checkpoints. Then I had a look at the proposed patch, with a couple of comments. - if (PriorRedoPtr == InvalidXLogRecPtr) - recycleSegNo = endlogSegNo + 10; - else - recycleSegNo = XLOGfileslop(PriorRedoPtr); + recycleSegNo = XLOGfileslop(RedoRecPtr); I think that this is a new behavior, and should not be changed, see point 3 below. In CreateCheckPoint(), the removal of past WAL segments is always going to happen as RedoRecPtr is never InvalidXLogRecPtr, so the recycling should happen also when PriorRedoPtr is InvalidXLogRecPtr, no? /* Trim from the last checkpoint, not the last - 1 */ This comment could be adjusted, let's remove "not the last - 1" . The calculation of _logSegNo in CreateRestartPoint is visibly incorrect, this still uses PriorRedoPtr so the bug is not fixed for standbys. The tweaks for ThisTimeLineID also need to be out of the loop where PriorRedoPtr is InvalidXLogRecPtr, and only the distance calculation should be kept. Finally, in summary, this patch is doing actually three things: 1) Rename a couple of variables which refer incorrectly to the prior checkpoint so as they refer to the last checkpoint generated. 2) Make sure that WAL recycling/removal happens based on the last checkpoint generated, which is the one just generated when past WAL segments are cleaned up as post-operation actions. 3) Enforce the recycling point to be the switch point instead of arbitrarily recycle 10 segments, which is what b2a5545b has introduced. Recycling at exactly the switch point is wrong, as the redo LSN of the previous checkpoint may not be at the same segment when a timeline has switched, so you would finish with recycling segments which are still needed if an instance needs be crash-recovered post-promotion without a completed post-recovery checkpoint. In short this is dangerous. I'll let Heikki comment on that, but I think that you should fetch the last redo LSN instead, still you need to be worried about checkpoints running in parallel of the startup process, so I would stick with the current logic. 1) and 2) are in my opinion clearly oversights from 4b0d28d, but 3) is not. -- Michael signature.asc Description: PGP signature
Re: Non-portable shell code in pg_upgrade tap tests
On Sun, Jul 22, 2018 at 02:53:51PM -0400, Tom Lane wrote: > Noah Misch writes: > > On Sun, Jul 22, 2018 at 10:46:03AM -0400, Tom Lane wrote: > >> The pg_upgrade makefile does in fact use $(SHELL), so it will default to > >> whatever shell configure used. > > > It will not, because we don't set $(SHELL) anywhere. $(SHELL) is not > > @SHELL@. > > In our makefiles, $(SHELL) is always /bin/sh, the GNU make default. > > Oh! Hm, I wonder whether we shouldn't do that, ie add SHELL = @SHELL@ > to Makefile.global.in. I see that as the right way forward.
[Proposal] Add accumulated statistics for wait event
Hello hackers,
This proposal is about recording additional statistics of wait events.
PostgreSQL statistics Issue
The pg_stat_activity view is very useful in analysis for performance issues.
But it is difficult to get information of wait events in detail,
when you need to deep dive into analysis of performance.
It is because pg_stat_activity just shows the current wait status of backend.
If PostgreSQL provides additional statistics about wait events,
it will be helpful in pinpointing the exact cause of throughput issue.
Proposal
Record additional statistics items per wait event for every backend.
- total elapsed time to wait
- max elapsed time to wait
- number of times being waited
I suggest storing the above statistics in the pgBackendStatus structure.
typedef struct PgBackendStatus
{
...
/*
* proc's wait_event additional information.
* each wait_events elapsed time & count.
*/
TimestampTz st_wait_event_start_timestamp;
uint64 st_wait_event_total_elapsed[NUM_WAIT_EVENT];
uint64 st_wait_event_max_elapsed[NUM_WAIT_EVENT];
uint32 st_wait_event_counting[NUM_WAIT_EVENT];
}
PoC test
I wrote a prototype patch.
With this patch, you can get additional wait event stats viathe new procedure ‘pg_stat_get_wait_events()’.
You can test by following steps.
1. apply git patch
- patch -p0 < wait_event_stat_patchfile.diff
2. make distclean
3. configure --with-wait-event-detail
4. make & make install
5. start postgreSQL and execute psql
6. using pg_stat_get_wait_events(null) function
- input parameter is pid.
display example>
postgres=# select * from pg_stat_get_wait_events(null) where counting > 0;
pid | wait_event_type | wait_event | total_elapsed | max_elapsed | counting
---+-+---+---+-+--
25291 | LWLock | WALBufMappingLock | 1359 | 376 |6
25291 | LWLock | WALWriteLock |679733 | 113803 |8
25291 | IO | BufFileRead | 780 | 7 | 172
25291 | IO | BufFileWrite | 1247 | 19 | 171
25291 | IO | DataFileExtend | 44703 | 53 | 3395
25291 | IO | DataFileImmediateSync |268798 | 72286 | 12
25291 | IO | DataFileRead | 91763 | 22149 | 30
25291 | IO | WALSync |441139 | 60456 | 28
25291 | IO | WALWrite | 9567 | 637 | 737
24251 | LWLock | WALBufMappingLock | 1256 | 350 |6
24251 | LWLock | WALWriteLock |649140 | 153994 |7
24251 | IO | BufFileRead | 620 | 9 | 172
24251 | IO | BufFileWrite | 1228 | 20 | 171
24251 | IO | DataFileExtend | 26884 | 51 | 3395
24251 | IO | DataFileImmediateSync |208630 | 21067 | 12
24251 | IO | DataFileRead |426278 | 17327 | 128
24251 | IO | WALSync |307055 | 70853 | 24
24251 | IO | WALWrite | 17935 | 961 | 2720
(18 rows)
etc. concept proposal
--
1. I allocated arrays for additional statistics per wait event.
Normally the backend doesn’t use all wait events.
So the size of memory used for recording statistics can be reduced
by allocating one hash list as memory pool for statistics of wait events.
2. This feature can be implemented as extension
if some hooks were provided in following functions,
- pgstat_report_wait_start
- Pgstat_report_wait_end
Feedback and suggestion will be very welcome.
Thanks!
wait_event_stat_patchfile.diff
Description: Binary data
Re: Memory leak with CALL to Procedure with COMMIT.
On Mon, Jul 23, 2018 at 12:19:12PM +0530, Prabhat Sahu wrote: > While testing with PG procedure, I found a memory leak on HEAD, with below > steps: > > postgres=# CREATE OR REPLACE PROCEDURE proc1(v1 INOUT INT) > AS $$ > BEGIN > commit; > END; $$ LANGUAGE plpgsql; > CREATE PROCEDURE > > postgres=# call proc1(10); > WARNING: Snapshot reference leak: Snapshot 0x23678e8 still referenced > v1 > > 10 > (1 row) I can reproduce this issue on HEAD and v11, so an open item is added. Peter, could you look at it? -- Michael signature.asc Description: PGP signature
Re: [HACKERS] Restricting maximum keep segments by repslots
Hello.
At Fri, 20 Jul 2018 10:13:58 +0900, Masahiko Sawada
wrote in
> > As I reconsidered this, I noticed that "lsn - lsn" doesn't make
> > sense here. The correct formula for the value is
> > "max_slot_wal_keep_size * 1024 * 1024 - ((oldest LSN to keep) -
> > restart_lsn). It is not a simple formula to write by hand but
> > doesn't seem general enough. I re-changed my mind to show the
> > "distance" there again.
> >
> > pg_replication_slots now has the column "remain" instaed of
> > "min_keep_lsn", which shows an LSN when wal_status is "streaming"
> > and otherwise "0/0". In a special case, "remain" can be "0/0"
> > while "wal_status" is "streaming". It is the reason for the
> > tristate return value of IsLsnStillAvaialbe().
> >
> > wal_status | remain
> > streaming | 0/19E3C0 -- WAL is reserved
> > streaming | 0/0 -- Still reserved but on the boundary
> > keeping| 0/0 -- About to be lost.
> > lost | 0/0 -- Lost.
> >
>
> The "remain" column still shows same value at all rows as follows
> because you always compare between the current LSN and the minimum LSN
> of replication slots. Is that you expected? My comment was to show the
Ouch! Sorry for the silly mistake. GetOldestKeepSegment should
calculate restBytes based on the distance from the cutoff LSN to
restart_lsn, not to minSlotLSN. The attached fixed v6 correctly
shows the distance individually.
> Also, I'm not sure it's a good way to show the distance as LSN. LSN is
> a monotone increasing value but in your patch, a value of the "remain"
> column can get decreased. As an alternative way I'd suggest to show it
The LSN of WAL won't be decreased but an LSN is just a position
in a WAL stream. Since the representation of LSN is composed of
the two components 'file number' and 'offset', it's quite natural
to show the difference in the same unit. The distance between the
points at "6m" and "10m" is "4m".
> as the number of segments. Attached patch is a patch for your v5 patch
> that changes it so that the column shows how many WAL segments of
> individual slots are remained until they get lost WAL.
Segment size varies by configuration, so segment number is not
intuitive to show distance. I think it is the most significant
reason we move to "bytes" from "segments" about WAL sizings like
max_wal_size. More than anything, it's too coarse. The required
segments may be lasts for the time to consume a whole segment or
may be removed just after. We could calculate the fragment bytes
but it requires some internal knowledge.
Instead, I made the field be shown in flat "bytes" using bigint,
which can be nicely shown using pg_size_pretty;
=# select pg_current_wal_lsn(), restart_lsn, wal_status, pg_size_pretty(remain)
as remain from pg_replication_slots ;
pg_current_wal_lsn | restart_lsn | wal_status | remain
+-++
0/DD3B188 | 0/CADD618 | streaming | 19 MB
0/DD3B188 | 0/DD3B188 | streaming | 35 MB
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 8cc6fe3106f58ae8cfe3ad8d4b25b5774ac6ec05 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi
Date: Thu, 21 Dec 2017 21:20:20 +0900
Subject: [PATCH 1/4] Add WAL releaf vent for replication slots
Adds a capability to limit the number of segments kept by replication
slots by a GUC variable.
---
src/backend/access/transam/xlog.c | 100 --
src/backend/utils/misc/guc.c | 12
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/access/xlog.h | 1 +
4 files changed, 94 insertions(+), 20 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 335b4a573d..4bf1536d8f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -105,6 +105,7 @@ int wal_level = WAL_LEVEL_MINIMAL;
int CommitDelay = 0; /* precommit delay in microseconds */
int CommitSiblings = 5; /* # concurrent xacts needed to sleep */
int wal_retrieve_retry_interval = 5000;
+int max_slot_wal_keep_size_mb = 0;
#ifdef WAL_DEBUG
bool XLOG_DEBUG = false;
@@ -867,6 +868,7 @@ static void checkTimeLineSwitch(XLogRecPtr lsn, TimeLineID newTLI,
static void LocalSetXLogInsertAllowed(void);
static void CreateEndOfRecoveryRecord(void);
static void CheckPointGuts(XLogRecPtr checkPointRedo, int flags);
+static XLogSegNo GetOldestKeepSegment(XLogRecPtr currpos, XLogRecPtr minSlotPtr);
static void KeepLogSeg(XLogRecPtr recptr, XLogSegNo *logSegNo);
static XLogRecPtr XLogGetReplicationSlotMinimumLSN(void);
@@ -9491,6 +9493,51 @@ CreateRestartPoint(int flags)
return true;
}
+/*
+ * Returns minimum segment number the next checktpoint must leave considering
+ * wal_keep_segments, replication slots and max_slot_wal_keep_size.
+ */
+static XLogSegNo
+GetOldestKeepSegment(XLogRecPtr currLSN, XLogRecPtr minSlotLSN)
+{
+ uint64 keepSegs = 0;
+ XLogSegNo cur
Re: [bug fix] Produce a crash dump before main() on Windows
On Mon, Feb 26, 2018 at 04:06:48AM +, Tsunakawa, Takayuki wrote: > As for PG11+, I agree that we want to always leave WER on. That is, > call SetErrorMode(SEM_FAILCRITICALERRORS) but not specify > SEM_NOGPFAULTERRORBOX. The problem with the current specification of > PostgreSQL is that the user can only get crash dumps in a fixed folder > $PGDATA\crashdumps. That location is bad because the crash dumps will > be backed up together with the database cluster without the user > noticing it. What's worse, the crash dumps are large. With WER, the > user can control the location and size of crash dumps. I have not looked by myself at the problems around WER and such so I don't have a clear opinion, but including crash dumps within backups is *bad*. We have a set of filtering rules in basebackup.c and pg_rewind since v11, we could make use of it and remove all contents from crashdumps/ from what gets backed up or rewound. excludeDirContents creates empty folders when those are listed, so we'd want to not create an empty folder in Windows backups, which makes a bit more more difficult. -- Michael signature.asc Description: PGP signature
Re: [bug fix] Produce a crash dump before main() on Windows
On Mon, Jul 23, 2018 at 01:31:57AM +, Tsunakawa, Takayuki wrote: > First, as Hari-san said, starting the server with "pg_ctl start" on > the command prompt does not feel like production use but like test > environment, and that usage seems mostly interactive. Second, the > current PostgreSQL is not exempt from the same problem: if postmaster > or standalone backend crashes before main(), the pop up would appear. When you use pg_ctl start within the command prompt, then closing the prompt session also causes Postgres to stop immediately. Would it be a problem? Another question I have is that I have seen Postgres fail to start because of missing dependent libraries, which happened after the postmaster startup as this was reported in the logs, could it be a problem with this patch? If the system waits for an operator to close this pop-up window, then it becomes harder to check automatically such failures and relies on the test application to timeout, if it is wise enough to do, which is not always the case.. -- Michael signature.asc Description: PGP signature
Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
(2018/07/21 0:26), Robert Haas wrote: On Fri, Jul 20, 2018 at 8:38 AM, Robert Haas wrote: This looks like a terrible design to me. If somebody in future invents a new plan type that is not projection-capable, then this could fail an assertion here and there won't be any simple fix. And if you reach here and the child targetlists somehow haven't been adjusted, then you won't even fail an assertion, you'll just crash (or maybe return wrong answers). To elaborate on this thought a bit, it is currently the case that all scan and join types are projection-capable, and most likely we would make any such node types added in the future projection-capable as well, but it still doesn't seem like a good idea to me to have tangentially-related parts of the system depend on that in subtle ways. I have to admit that that is not a good idea. So, I'll update the patch so that we don't assume the projection capability of the subplan anymore, if we go this way. Also, even today, this would fail if the subplan happened to be a Sort, and it's not very obvious why that couldn't happen. You mean the MergeAppend case? In that case, the patch will adjust the subplan before adding a Sort above the subplan, so that could not happen. More broadly, the central idea of this patch is that we can set the target list for a child rel to something other than the target list that we expect it will eventually produce, and then at plan creation time fix it. Yeah, the patch would fix the the final output to the appendrel parent at plan creation time if necessary, but it would produces the tlist of an intermediate child scan/join node as written in the child relation's reltarget at that time. I think that's a bad idea. The target list affects lots of things, like costing. If we don't insert a ConvertRowTypeExpr into the child's target list, the costing will be wrong to the extent that ConvertRowTypeExpr has any cost, which it does. Actually, this is not true at least currently, because set_append_rel_size doesn't do anything about the costing: * NB: the resulting childrel->reltarget->exprs may contain arbitrary * expressions, which otherwise would not occur in a rel's targetlist. * Code that might be looking at an appendrel child must cope with * such. (Normally, a rel's targetlist would only include Vars and * PlaceHolderVars.) XXX we do not bother to update the cost or width * fields of childrel->reltarget; not clear if that would be useful. Any other current or future property that is computed based on the target list -- parallel-safety, width, security-level, whatever -- could also potentially end up with a wrong value if the target list as written does not match the target list as will actually be produced. It's true that, in all of these areas, the ConvertRowTypeExpr isn't likely to have a lot of impact; it probably won't have a big effect on costing and may not actually cause a problem for the other things that I mentioned. On the other hand, if it does cause a serious problem, what options would we have for fixing it other than to back out your entire patch and solve the problem some other way? The idea that derived properties won't be correct unless they are based on the real target list is pretty fundamental, and I think deviating from the idea that we get the correct target list first and then compute the derived properties afterward is likely to cause real headaches for future developers. In short, plan creation time is just the wrong time to change the plan. It's just a time to translate the plan from the form needed by the planner to the form needed by the executor. It would be fine if the change you were making were only cosmetic, but it's not. Some createplan.c routines already change the tlists of their nodes. For example, create_merge_append_plan would add sort columns to each of its subplans if necessary. I think it would be similar to that to add a ConvertRowtypeExpr above a child whole-row Var in the subplan's tlist. After looking over both patches, I think Ashutosh Bapat has basically the right idea. What he is proposing is a localized fix that doesn't seem to make any changes to the way things work overall. I reviewed his patch, and thought that that would fix the issue, but this is about the current design on the child tlist handling in partitionise join rather than that patch: it deviates from the assumption that we had for the scan/join planning till PG10 that "a rel's targetlist would only include Vars and PlaceHolderVars", and turns out that there are a lot of places where we need to modify code to suppress that deviation, as partly shown in that patch. So, ISTM that the current design in itself is not that localized. Partition-wise join/aggregate, and partitioning in general, need a lot of optimization in a lot of places, and fortunately there are people working on that, but our goal her
Re: [Proposal] Add accumulated statistics for wait event
On Mon, Jul 23, 2018 at 04:04:42PM +0900, 임명규 wrote: > This proposal is about recording additional statistics of wait events. You should avoid sending things in html format, text format being recommended on those mailing lists... The patch applies after using patch -p0 by the way. I would recommend that you generate your patches using "git format-patch". Here are general guidelines on the matter: https://wiki.postgresql.org/wiki/Submitting_a_Patch Please study those guidelines, those are helpful if you want to get yourself familiar with community process. I have comments about your patch. First, I don't think that you need to count precisely the number of wait events triggered as usually when it comes to analyzing a workload's bottleneck what counts is a periodic *sampling* of events, patterns which can be fetched already from pg_stat_activity and stored say in a different place. This can be achieved easily by using a cron job with an INSERT SELECT query which adds data on a relation storing the event counts. I took the time to look at your patch, and here is some feedback. This does not need a configure switch. I assume that what you did is good to learn the internals of ./configure though. There is no documentation. What does the patch do? What is it useful for? + case PG_WAIT_IPC: + arrayIndex = NUM_WAIT_LWLOCK + + NUM_WAIT_LOCK + NUM_WAIT_BUFFER_PIN + + NUM_WAIT_ACTIVITY + NUM_WAIT_CLIENT + + NUM_WAIT_EXTENSION + eventId; + break; This is ugly and unmaintainable style. You could perhaps have considered an enum instead. pg_stat_get_wait_events should be a set-returning function, which you could filter at SQL level using a PID, so no need of it as an argument. What's the performance penalty? I am pretty sure that this is measurable as wait events are stored for a backend for each I/O operation as well, and you are calling a C routine within an inlined function which is designed to be light-weight, doing only a four-byte atomic operation. -- Michael signature.asc Description: PGP signature
RE: [bug fix] Produce a crash dump before main() on Windows
From: Michael Paquier [mailto:mich...@paquier.xyz] > When you use pg_ctl start within the command prompt, then closing the prompt > session also causes Postgres to stop immediately. Would it be a problem? No. But that makes me think more of the startup on command prompt as non-production usage. > Another question I have is that I have seen Postgres fail to start because > of missing dependent libraries, which happened after the postmaster startup > as this was reported in the logs, could it be a problem with this patch? > If the system waits for an operator to close this pop-up window, then it > becomes harder to check automatically such failures and relies on the test > application to timeout, if it is wise enough to do, which is not always > the case.. I guess that is due to some missing files related to the libraries listed in shared_preload_library. If so, no, because this patch relates to failure before main(). Regards Takayuki Tsunakawa
Re: [Proposal] Add accumulated statistics for wait event
> This proposal is about recording additional statistics of wait events. > The pg_stat_activity view is very useful in analysis for performance > issues. > But it is difficult to get information of wait events in detail, > when you need to deep dive into analysis of performance. > It is because pg_stat_activity just shows the current wait status of > backend. There is an extension that samples the information from pg_stat_activity (similar to Oracle's ASH). https://github.com/postgrespro/pg_wait_sampling Maybe it's worthwhile to combine the efforts? -- Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
pgcrypto: is PGP_PUB_DSA_SIGN used?
Hi hackers, I'm trying to look how to trigger code paths under PGP_PUB_DSA_SIGN constant, and it seems like it is a dead code in the current implementation. Can anyone confirm this, or help me with creating a correct DSA sign-only key with RSA encryption subkey that would trigger the code paths? Pavel
Re: [HACKERS] advanced partition matching algorithm for partition-wise join
On Fri, Jul 20, 2018 at 3:13 AM, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > It's of course wrong, it's going to be O(max(m, n)) as you said, but > the point is still valid - if we have partitions A1, A2 from one side > and B1, ..., BN on another side, we can skip necessary the > partitions from B that are between e.g. A1 and A2 faster with > binary search. That's possible only when there is no default partition and the join is an inner join. For an outer join, we need to include all the partitions on the outer side, so we can't just skip over some partitions. In case of a default partition, it can take place of a missing partition, so we can't skip partitions using binary search. The code right now works for all the cases and is O(n). I agree that it can be optimized for certain cases, but 1. those cases are rare enough that we can ignore those right now. How many times we would encounter the case you have quoted, for example? Usually the ranges will be continuous only differing in the first or last partition e.g time-series data. 2. The code is enough complex right now and it's also a lot. Making it complicated further is not the worth the rare use cases. If we get complaints from the field, we can certainly improve it in future. But I would wait for those complaints before improving it further. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Re: [Proposal] Add accumulated statistics for wait event
Hi, that will be a great feature. On 23.07.2018 10:53, Michael Paquier wrote: I have comments about your patch. First, I don't think that you need to count precisely the number of wait events triggered as usually when it comes to analyzing a workload's bottleneck what counts is a periodic *sampling* of events If I get it right, this patch is not about sampling. It gathers cumulative statistics, which is regrettably missing in PostgreSQL. That's why it should not only count exact number of wait events, but also min time and stddev would be helpful (as in pg_stat_statements). -- Egor Rogov Postgres Professional http://www.postgrespro.com
Re: [bug fix] Produce a crash dump before main() on Windows
On Mon, Jul 23, 2018 at 08:16:52AM +, Tsunakawa, Takayuki wrote: > I guess that is due to some missing files related to the libraries > listed in shared_preload_library. If so, no, because this patch > relates to failure before main(). No, I really mean a library dependency failure. For example, imagine that Postgres is compiled on Windows dynamically, and that it depends on libxml2.dll, which is itself compiled dynamically. Then imagine, in a custom build echosystem, that a folk comes in and adds lz support to libxml2 on Windows. If Postgres still consumes libxml2 but does not add in its PATH a version of lz, then a backend in need of libxml2 would fail to load, causing Postgres to not start properly. True, painful, story. What is ticking me is if the popup window stuff discussed on this thread could be a problem in the detection of such dependency errors, as with the product I am thinking about, Postgres is not running as a service, but kicked by another thing which is a service, and monitors the postmaster. -- Michael signature.asc Description: PGP signature
Re: de-deduplicate code in DML execution hooks in postgres_fdw
On Fri, Jul 20, 2018 at 2:27 PM, Etsuro Fujita wrote: > (2018/07/20 13:49), Michael Paquier wrote: >> >> On Thu, Jul 19, 2018 at 05:35:11PM +0900, Etsuro Fujita wrote: >>> >>> +1 for the general idea. (Actually, I also thought the same thing >>> before.) >>> But since this is definitely a matter of PG12, ISTM that it's wise to >>> work >>> on this after addressing the issue in [1]. My concern is: if we do this >>> refactoring now, we might need two patches for fixing the issue in case >>> of >>> backpatching as the fix might need to change those executor functions. >> >> >> FWIW, I would think that if some cleanup of the code is obvious, we >> should make it without waiting for the other issues to settle down >> because there is no way to know when those are done, > > > I would agree to that if we were late in the development cycle for PG12. > >> and this patch >> could be forgotten. > > > I won't forget this patch. :) > >> Looking at the proposed patch, moving the new routine closer to >> execute_dml_stmt and renaming it execute_dml_single_row would be nicer. > > > Or execute_parameterized_dml_stmt? I have placed it closer to its caller, I think that's better than moving it closer to execute_dml_stmt, unless there is any other strong advantage. I used perform instead of execute since the later is usually associated with local operation. I added "foreign" in the name of the function to indicate that it's executed on foreign server. I am happy with "remote" as well. I don't think "one" and "single" make any difference. I don't like "parameterized" since that gets too tied to the method we are using rather than what's actually being done. In short I don't find any of the suggestions to be significantly better or worse than the name I have chosen. Said that, I am not wedded to any of those. A committer is free to choose anything s/he likes. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Re: [HACKERS] Two pass CheckDeadlock in contentent case
On Sat, Jul 21, 2018 at 2:58 AM, Yura Sokolov wrote: > > It is regular pgbench output, so there is no source for confusion. > tps numbers is number of transactions completed in that particular > 5 second interval. That is why there are 0 tps and 1 tps intervals > without patch. Which way 0tps and 1tps could be if tps were average > since start? > In your output there's no mention of 0s and 1s tps. It might be something standard with pgbench, which I don't know yet. Sorry for that. > It means that without patch there were > 46k+51k+52k+47k+42k+12k+0k+0k+0k+0k+27k+39k+41k=357 thousands of > transactions completed. > > And with patch there were > 56k+55k+51k+51k+48k+45k+40k+14k+30k+39k+41k+38k+40k+38k=586 thousands of > transactions completed. Thanks for the explanation. > >> Talking about GUCs, deadlock_timeout [1] value decides when the >> deadlock check kicks in. In such cases probably increasing it would >> suffice and we may not need the patch. > > Increasing deadlock_timeout solves this issue. > But increasing deadlock timeout is very-very bad decision in presence of > real deadlocks. I thought you were trying to improve a situation where the transactions are genuinely taking longer than deadlock timeout and ending up with deadlock detection unnecessarily. For that case the advice that deadlock_timeout should be set to a value longer than usual transaction period works. In case of real deadlocks, I think that penalizes the deadlocking backends by making them wait longer after the actual deadlock happens. But in such case, the solution is to avoid such deadlock at the level of application or in the backend (deadlock with system tables involved). Said that your patch is small enough to accept if it improves those cases as well. > >>> Excuse me, corrected version is in attach. >>> >> >> About the patch itself, I think releasing the shared lock and taking >> exclusive lock on lock partitions, may lead to starvation. If there >> are multiple backends that enter deadlock detection simultaneously, >> all of them will find that there's a deadlock and one of them will >> succeed in resolving it only after all of them release their >> respective locks. Also there's a comment at the end about the order in >> which to release the lock > > It is not worse than behavior without patch. Without patch when real > deadlock happends and multiple backends enters deadlock detection, all > of them (except first) will wait for first backend to release all > EXCLUSIVE locks only to lock them again and then to find that deadlock > eliminated. > Given there is N backends in deadlock, there will be 1*X+(N-1)*Y=Q total > time without patch, where X is time to detect and resolve deadlock, and > Y is a time to check for deadlock after resolving. > > With patch backends will do first pass in parallel, so it will be > Z*(1+e)+Q total time, where Z is a time to make first fast imprecise > check, and e is a time drift due to process scheduling. > Note, that in reality all this times usually much smaller than 1 second > default for deadlock_timeout, so that backends will wait for > deadlock_timeout much longer that additional Z*(1+e) time need for first > inexact pass. > And, if some backends tries to detect deadlock a bit later, ie after one > backend already takes exclusive lock, I think the case I am talking about is somewhere between these two - the backend which has imprecisely decided that there's a deadlock will start taking exclusive locks and will have to wait for all the backends with shared locks to release there locks. This will never happen if there is a stream of backends which keeps on taking shared locks. This is classical problem of starvation because of lock upgrade. Taking exclusive lock to begin with avoids this problem. >> >> /* >> * And release locks. We do this in reverse order for two reasons: (1) >> * Anyone else who needs more than one of the locks will be trying to >> lock >> * them in increasing order; we don't want to release the other process >> * until it can get all the locks it needs. Your patch is breaking this assumption. We may end up in a situation where more than two backends have shared locks. One of the backend starts releasing its locks and releases even the first lock. Some different backend gets its first exclusive lock because of that but can not get the next one since it's held by the second backend holding shared locks. This might not happen if by "more than one" the comment means all the locks. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
cached plans and enable_partition_pruning
It seems that because enable_partition_pruning's value is only checked
during planning, turning it off *after* a plan is created and cached does
not work as expected.
create table p (a int) partition by list (a);
create table p1 partition of p for values in (1);
create table p1 partition of p for values in (2);
-- force a generic plan so that run-time pruning is used in the plan
reset enable_partition_pruning;
set plan_cache_mode to force_generic_plan;
prepare p as select * from p where a = $1;
explain (costs off, analyze) execute p (1);
QUERY PLAN
Append (actual time=0.079..0.106 rows=1 loops=1)
Subplans Removed: 1
-> Seq Scan on p2 (actual time=0.058..0.068 rows=1 loops=1)
Filter: (a = $1)
Planning Time: 17.573 ms
Execution Time: 0.396 ms
(6 rows)
set enable_partition_pruning to off;
explain (costs off, analyze) execute p (1);
QUERY PLAN
Append (actual time=0.108..0.135 rows=1 loops=1)
Subplans Removed: 1
-> Seq Scan on p2 (actual time=0.017..0.028 rows=1 loops=1)
Filter: (a = $1)
Planning Time: 0.042 ms
Execution Time: 0.399 ms
(6 rows)
Pruning still occurs, whereas one would expect it not to, because the plan
(the Append node) contains run-time pruning information, which was
initialized because enable_partition_pruning was turned on when the plan
was created.
Should we check its value during execution too, as done in the attached?
Thanks,
Amit
diff --git a/src/backend/executor/nodeAppend.c
b/src/backend/executor/nodeAppend.c
index 1fc55e90d7..d67abed330 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -61,6 +61,7 @@
#include "executor/execPartition.h"
#include "executor/nodeAppend.h"
#include "miscadmin.h"
+#include "optimizer/cost.h"
/* Shared state for parallel-aware Append. */
struct ParallelAppendState
@@ -128,8 +129,12 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
/* Let choose_next_subplan_* function handle setting the first subplan
*/
appendstate->as_whichplan = INVALID_SUBPLAN_INDEX;
- /* If run-time partition pruning is enabled, then set that up now */
- if (node->part_prune_info != NULL)
+ /*
+* If run-time partition pruning is enabled, then set that up now.
+* However, enable_partition_pruning may have been turned off since when
+* the plan was created, so recheck its value.
+*/
+ if (node->part_prune_info != NULL && enable_partition_pruning)
{
PartitionPruneState *prunestate;
diff --git a/src/backend/executor/nodeMergeAppend.c
b/src/backend/executor/nodeMergeAppend.c
index c7eb18e546..5252930b17 100644
--- a/src/backend/executor/nodeMergeAppend.c
+++ b/src/backend/executor/nodeMergeAppend.c
@@ -43,6 +43,7 @@
#include "executor/nodeMergeAppend.h"
#include "lib/binaryheap.h"
#include "miscadmin.h"
+#include "optimizer/cost.h"
/*
* We have one slot for each item in the heap array. We use SlotNumber
@@ -89,8 +90,12 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int
eflags)
mergestate->ps.ExecProcNode = ExecMergeAppend;
mergestate->ms_noopscan = false;
- /* If run-time partition pruning is enabled, then set that up now */
- if (node->part_prune_info != NULL)
+ /*
+* If run-time partition pruning is enabled, then set that up now.
+* However, enable_partition_pruning may have been turned off since when
+* the plan was created, so recheck its value.
+*/
+ if (node->part_prune_info != NULL && enable_partition_pruning)
{
PartitionPruneState *prunestate;
Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
On Fri, Jul 20, 2018 at 8:56 PM, Robert Haas wrote: [ ... clipped portion ...] > > In short, plan creation time is just the wrong time to change the > plan. It's just a time to translate the plan from the form needed by > the planner to the form needed by the executor. It would be fine if > the change you were making were only cosmetic, but it's not. > Agree with all that, including the clipped paras. > After looking over both patches, I think Ashutosh Bapat has basically > the right idea. What he is proposing is a localized fix that doesn't > seem to make any changes to the way things work overall. I do think > his patches need to be fixed up a bit to avoid conflating > ConvertRowtypeExpr with "converted whole-row reference." The two are > certainly not equivalent; the latter is a narrow special case. Some > of the comments could use some more wordsmithing, too, I think. Apart > from those gripes, though, I think it's solving the problem the > correct way: don't build the wrong plan and try to fix it, just build > the right plan from the beginning. I will work on that if everyone agrees that that's the right way to go. Fujita-san seems to have some arguments still. I have argued on the same lines as yours but your explanation is better. I don't have anything more to add. I will wait for that to be resolved. > > There are definitely some things not to like about this approach. In > particular, I definitely agree that treating a converted whole-row > reference specially is not very nice. It feels like it might be > substantially cleaner to be able to represent this idea as a single > node rather than a combination of ConvertRowtypeExpr and var with > varattno = 0. Perhaps in the future we ought to consider either (a) > trying to use a RowExpr for this rather than ConvertRowtypeExpr or (b) > inventing a new WholeRowExpr node that stores two RTIs, one for the > relation that's generating the whole-row reference and the other for > the relation that is controlling the column ordering of the result or > (c) allowing a Var to represent a whole-row expression that has to > produce its outputs according to the ordering of some other RTE. I never liked representing a whole-row expression as a Var worst with varattno = 0. We should have always casted it as RowExpr(set of vars, one var per column). This problem wouldn't arise then. Many other problems wouldn't arise then, I think. I think we do that so that we can convert a tuple from buffer into a datum and put it in place of Var with varattno = 0. Given that I prefer (a) or (b) in all the cases. If not that, then c. But I agree that we have to avoid ConvertRowtypeExpr being used in this case. > But > I don't think it's wise or necessary to whack that around just to fix > this bug; it is refactoring or improvement work best left to a future > release. > > Also, it is definitely a real shame that we have to build attr_needed > data for each child separately. Right now, we've got to build a whole > lot of things for each child individually, and that's slow and > inefficient. We're not going to scale nicely to large partitioning > hierarchies unless we can figure out some way of minimizing the work > we do for each partition. So, the fact that Fujii-san's approach > avoids needing to compute attr_needed in some cases is very appealing. > However, in my opinion, it's nowhere near appealing enough to justify > trying to do surgery on the target list at plan-creation time. I > think we need to leave optimizing this to a future effort. > Partition-wise join/aggregate, and partitioning in general, need a lot > of optimization in a lot of places, and fortunately there are people > working on that, but our goal here should just be to fix things up > well enough that we can ship it. I agree. > I don't see anything in Ashutosh's > patch that is so ugly that we can't live with it for the time being. Thanks. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Re: Log query parameters for terminated execute
On Sun, Jun 24, 2018 at 1:08 AM, Pavel Stehule wrote: > > > 2018-06-23 21:54 GMT+02:00 Sergei Kornilov : > >> Hello all >> We already have feature to logging query parameters. If we use >> log_statement = 'all' we write parameters before execution and all is fine >> here. If we use log_min_duration_statement statement is logged obviously >> after execution, but currently we have no parameters if query was >> terminated by statement_timeout, lock_timeout or by pg_terminate_backend. >> >> I would like have parameters in logs at least for such three cases. >> > > It is good idea - more times I would to see these values > > Regards > > Pavel > > >> Simple way achieve this is just add errdetail_params to such ereport as >> in attached patch. >> Another way is add something like printing global variable >> debug_query_string in send_message_to_server_log >> (src/backend/utils/error/elog.c). But i have no good idea how print >> ParamListInfo correctly. We can not use OidOutputFunctionCall in all cases, >> right? >> >> Another small question is why errdetail_params uses errdetail instead >> errdetail_log? We assume that the user wants to get their own parameters >> back (if he set client_min_messages to LOG)? >> >> Any feedback is strongly appreciated. Thank you! >> >> regards, Sergei > > > I have reviewed and tested the patch here are my findings about the patch. Patch applied successfully on master branch "04269320aed30d3e37c10ae5954eae234d45". There is no compilation issue with the patch. statement_timeout: For this I wrote a simple LibPq program to insert into table. The results are random, some times it logs the param and some time does not; with the same program. After digging a bit I found that if you execute just after starting the server it does not log the param and start logging after subsequent calls. Here is the log 2018-07-23 14:12:13.530 PKT [29165] ERROR: canceling statement due to statement timeout 2018-07-23 14:12:13.530 PKT [29165] DETAIL: parameters: $1 = '16777216', $2 = 'Foobar' 2018-07-23 14:12:13.530 PKT [29165] STATEMENT: INSERT INTO tbl_libpq_test (id, name) VALUES ($1::integer, $2::varchar) ... 2018-07-23 14:13:38.483 PKT [29169] LOG: shutting down ... 2018-07-23 14:13:38.616 PKT [29901] LOG: database system is ready to accept connections 2018-07-23 14:13:39.848 PKT [29910] ERROR: canceling statement due to statement timeout 2018-07-23 14:13:39.848 PKT [29910] STATEMENT: INSERT INTO tbl_libpq_test (id, name) VALUES ($1::integer, $2::varchar) lock_timeout: For this I a use the same program to insert into table. I lock the table in other session and try to execute the program. It does not log any params at all here is the log. 2018-07-23 14:21:19.165 PKT [30006] ERROR: canceling statement due to lock timeout at character 13 2018-07-23 14:21:19.165 PKT [30006] STATEMENT: INSERT INTO tbl_libpq_test (id, name) VALUES ($1::integer, $2::varchar) -- Ibrar Ahmed
Re: Log query parameters for terminated execute
Hello Thank you for review! Well, i can miss some cases. I'm not sure about overall design of this patch. Is acceptable add errdetail_params to statement_timeout ereport in such way? After shutdown signal we must be in aborted state, so we mustn't call user-defined I/O functions. (quotation from comment to errdetail_params in src/backend/tcop/postgres.c ). It seems i can not fix it with current design. > ERROR: canceling statement due to lock timeout at character 13 Hm, "at character"? How can we get this message? I found only "canceling statement due to lock timeout" (without "at character") ereport in src/backend/tcop/postgres.c Maybe try .. catch in parse state, not in execute? regards, Sergei
Re: Possible performance regression in version 10.1 with pgbench read-write tests.
On Mon, Jul 23, 2018 at 3:40 PM, Thomas Munro wrote: > On Sun, Jul 22, 2018 at 8:19 AM, Tom Lane wrote: >> 8 clients 72 clients >> >> unmodified HEAD 16112 16284 >> with padding patch 16096 16283 >> with SysV semas 15926 16064 >> with padding+SysV 15949 16085 >> >> This is on RHEL6 (kernel 2.6.32-754.2.1.el6.x86_64), hardware is dual >> 4-core Intel E5-2609 (Sandy Bridge era). This hardware does show NUMA >> effects, although no doubt less strongly than Mithun's machine. >> >> I would like to see some other results with a newer kernel. I tried to >> repeat this test on a laptop running Fedora 28, but soon concluded that >> anything beyond very short runs was mainly going to tell me about thermal >> throttling :-(. I could possibly get repeatable numbers from, say, >> 1-minute SELECT-only runs, but that would be a different test scenario, >> likely one with a lot less lock contention. > > I did some testing on 2-node, 4-node and 8-node systems running Linux > 3.10.something (slightly newer but still ancient). Only the 8-node > box (= same one Mithun used) shows the large effect (the 2-node box > may be a tiny bit faster patched but I'm calling that noise for now... > it's not slower, anyway). Here's an attempt to use existing style better: a union, like LWLockPadded and WALInsertLockPadded. I think we should back-patch to 10. Thoughts? -- Thomas Munro http://www.enterprisedb.com 0001-Pad-semaphores-to-avoid-false-sharing.patch Description: Binary data
Re: Log query parameters for terminated execute
On Mon, Jul 23, 2018 at 3:05 PM, Sergei Kornilov wrote: > Hello > Thank you for review! > Well, i can miss some cases. I'm not sure about overall design of this > patch. Is acceptable add errdetail_params to statement_timeout ereport in > such way? > > After shutdown signal we must be in aborted state, so we mustn't call > user-defined I/O functions. (quotation from comment to errdetail_params in > src/backend/tcop/postgres.c ). It seems i can not fix it with current > design. > No its not about calling the function after abort/shutdown. Just start the server and try to run the program, most of the time you will not get params. > > > ERROR: canceling statement due to lock timeout at character 13 > Hm, "at character"? How can we get this message? I found only "canceling > statement due to lock timeout" (without "at character") ereport in > src/backend/tcop/postgres.c > Maybe try .. catch in parse state, not in execute? > Its really easy to reproduce, just lock the table form another session and run a "c" program to insert row in the same table. > > regards, Sergei > -- Ibrar Ahmed
Re: Log query parameters for terminated execute
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested Review submitted The new status of this patch is: Waiting on Author
Re: JIT breaks PostGIS
Hello, пн, 23 июл. 2018 г. в 8:13, Andres Freund : > Hi, > > On 2018-07-21 23:14:47 +0300, Darafei "Komяpa" Praliaskouski wrote: > > > > I suspect that a fix would require to bisect llvm/clang version which > stops > > showing this behavior and making it a new minimum for JIT, if this is > not a > > symptom of bigger (memory management?) problem. > > It looks to me like it's a LLVM issue, specifically > https://bugs.llvm.org/show_bug.cgi?id=34424 > fixed in LLVM 5+. > Thank you for your investigation. > It'll only be an issue for extensions that throw c++ style exceptions. I > don't think that rises to the level of disallowing any LLVM version < > 5.0. I suggest postgis adds an error check to its buildprocess that > refuses to run if jit is enabled and a too old version is used? > Unfortunately that's not an option. Debian Stretch amd64 is quite popular platform, and is supported by Postgres 10 / PostGIS 2.4. If Christoph decides to keep LLVM enabled for 11 on that platform, as he is recommended upthread by Tom, that would mean that PostGIS can't be packaged at all, and all the people who need it will have to stay on Postgres 10. If PostGIS decides not to implement the check, and instead tweaks test runner to execute `set jit to off;` before tickets.sql, then Postgres 11 on that platform will have a known way to segfault it, even without superuser rights, as jit GUC is tweakable by anyone. I think that a good way to deal with it will be to bump minimum required version of LLVM to 5 on Postgres build, with a notice in docs that will say that "you can build with lower version, but that will give you this and this bug". It also may happen that a patch for LLVM can be applied to LLVM4 build in Debian and brought in as an update, but such a plan should not be a default one. -- Darafei Praliaskouski Support me: http://patreon.com/komzpa
Re: Fix calculations on WAL recycling.
At Mon, 23 Jul 2018 15:59:16 +0900, Michael Paquier wrote in <20180723065916.gi2...@paquier.xyz> > On Mon, Jul 23, 2018 at 01:57:48PM +0900, Kyotaro HORIGUCHI wrote: > > I'll register this to the next commitfest. > > > > https://www.postgresql.org/message-id/20180719.125117.155470938.horiguchi.kyot...@lab.ntt.co.jp > > This is an open item for v11. Mmm. Thanks. I wrongly thought this was v10 item. Removed this from the next CF. Thank you for taking a look. > >> While considering this, I found a bug in 4b0d28de06, which > >> removed prior checkpoint from control file. It actually trims the > >> segments before the last checkpoint's redo segment but recycling > >> is still considered based on the *prevous* checkpoint. As the > >> result min_wal_size doesn't work as told. Specifically, setting > >> min/max_wal_size to 48MB and advance four or more segments then > >> two checkpoints leaves just one segment, which is less than > >> min_wal_size. > >> > >> The attached patch fixes that. One arguable point on this would > >> be the removal of the behavior when RemoveXLogFile(name, > >> InvalidXLogRecPtr, ..). > >> > >> The only place calling the function with the parameter is > >> timeline switching. Previously unconditionally 10 segments are > >> recycled after switchpoint but the reason for the behavior is we > >> didn't have the information on previous checkpoint at hand at the > >> time. But now we can use the timeline switch point as the > >> approximate of the last checkpoint's redo point and this allows > >> us to use min/max_wal_size properly at the time. > > I have been looking at that, and tested with this simple scenario: > create table aa (a int); > > Then just repeat the following SQLs: > insert into aa values (1); > select pg_switch_wal(); > checkpoint; > select pg_walfile_name(pg_current_wal_lsn()); > > By doing so, you would notice that the oldest WAL segment does not get > recycled after the checkpoint, while it should as the redo pointer is > always checkpoint generated. What happens is that this oldest segment > gets recycled every two checkpoints. (I'm not sure I'm getting it correctly..) I see the old segments recycled. When I see pg_switch_wal() returns 0/12.., pg_walfile_name is ...13 and segment files for 13 and 14 are found in pg_wal directory. That is, seg 12 was recycled as seg 14. log_checkpoint=on shows every checkpoint recycles 1 segment in the case. > Then I had a look at the proposed patch, with a couple of comments. > > - if (PriorRedoPtr == InvalidXLogRecPtr) > - recycleSegNo = endlogSegNo + 10; > - else > - recycleSegNo = XLOGfileslop(PriorRedoPtr); > + recycleSegNo = XLOGfileslop(RedoRecPtr); > I think that this is a new behavior, and should not be changed, see > point 3 below. > > In CreateCheckPoint(), the removal of past WAL segments is always going > to happen as RedoRecPtr is never InvalidXLogRecPtr, so the recycling > should happen also when PriorRedoPtr is InvalidXLogRecPtr, no? Yes. The reason for the change was the change of RemoveNonParentXlogFiles that I made and it is irrelevant to this bug fix (and rather breaking as you pointed..). So, reverted it. > /* Trim from the last checkpoint, not the last - 1 */ > This comment could be adjusted, let's remove "not the last - 1" . Oops! Thanks. The comment has finally vanished and melded into another comment just above. | * Delete old log files not required by the last checkpoint and recycle | * them > The calculation of _logSegNo in CreateRestartPoint is visibly incorrect, > this still uses PriorRedoPtr so the bug is not fixed for standbys. The > tweaks for ThisTimeLineID also need to be out of the loop where > PriorRedoPtr is InvalidXLogRecPtr, and only the distance calculation > should be kept. Agreed. While I reconsidered this, I noticed that the estimated checkpoint distance is 0 when PriorRedoPtr is invalid. This means that the first checkpoint/restartpoint tries to reduce WAL segments to min_wal_size. However, it happens only at initdb time and makes no substantial behavior change so I made the change ignoring the difference. > Finally, in summary, this patch is doing actually three things: > 1) Rename a couple of variables which refer incorrectly to the prior > checkpoint so as they refer to the last checkpoint generated. > 2) Make sure that WAL recycling/removal happens based on the last > checkpoint generated, which is the one just generated when past WAL > segments are cleaned up as post-operation actions. > 3) Enforce the recycling point to be the switch point instead of > arbitrarily recycle 10 segments, which is what b2a5545b has introduced. > Recycling at exactly the switch point is wrong, as the redo LSN of the > previous checkpoint may not be at the same segment when a timeline has > switched, so you would finish with recycling segments which are still > needed if an instance needs be crash-recovered post-promotion without > a completed post-recovery checkpoint
Re: [Proposal] Add accumulated statistics for wait event
On Mon, Jul 23, 2018 at 10:53 AM Michael Paquier wrote: > What's the performance penalty? I am pretty sure that this is > measurable as wait events are stored for a backend for each I/O > operation as well, and you are calling a C routine within an inlined > function which is designed to be light-weight, doing only a four-byte > atomic operation. Yes, the question is overhead of measuring durations of individual wait events. It has been proposed before, and there been heated debates about that (see threads [1-3]). It doesn't seem to be a conclusion about this feature. The thing to be said for sure: performance penalty heavily depends on OS/hardware/workload. In some cases overhead is negligible, but in other cases it appears to be huge. 1. https://www.postgresql.org/message-id/flat/559D4729.9080704%40postgrespro.ru 2. https://www.postgresql.org/message-id/flat/CA%2BTgmoYd3GTz2_mJfUHF%2BRPe-bCy75ytJeKVv9x-o%2BSonCGApw%40mail.gmail.com 3. https://www.postgresql.org/message-id/flat/CAG95seUAQVj09KzLwU%2Bz1B-GqdMqerzEkPFR3hn0q88XzMq-PA%40mail.gmail.com -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [HACKERS] logical decoding of two-phase transactions
Hi Andres, >> > That'd require that an index lookup can't crash if the corresponding >> > heap entry doesn't exist (etc), but that's something we need to handle >> > anyway. The issue that multiple separate catalog lookups need to be >> > coherent (say Robert's pg_class exists, but pg_attribute doesn't >> > example) is solved by virtue of the the pg_attribute lookups failing if >> > the transaction aborted. > Not quite, no. Basically, in a simplified manner, the logical decoding > loop is like: > > while (true) > record = readRecord() > logical = decodeRecord() > > PG_TRY(): > StartTransactionCommand(); > > switch (TypeOf(logical)) > case INSERT: > insert_callback(logical); > break; > ... > > CommitTransactionCommand(); > > PG_CATCH(): > AbortCurrentTransaction(); > PG_RE_THROW(); > > what I'm proposing is that that various catalog access functions throw a > new class of error, something like "decoding aborted transactions". When will this error be thrown by the catalog functions? How will it determine that it needs to throw this error? > PG_CATCH(): > AbortCurrentTransaction(); > if (errclass == DECODING_ABORTED_XACT) >in_progress_xact_abort_pending = true; >continue; > else >PG_RE_THROW(); > > Now obviously that's just pseudo code with lotsa things missing, but I > think the basic idea should come through? > How do we handle the cases where the catalog returns inconsistent data (without erroring out) which does not help with the ongoing decoding? Consider for example: BEGIN; /* CONSIDER T1 has one column C1 */ ALTER TABLE T1 ADD COL c2; INSERT INTO TABLE T1(c2) VALUES; PREPARE TRANSACTION; If we abort the above 2PC and the catalog row for the ALTER gets cleaned up by vacuum, then the catalog read will return us T1 with one column C1. The catalog scan will NOT error out but will return metadata which causes the insert-decoding change apply callback to error out. The point here is that in some cases the catalog scan might not error out and might return inconsistent metadata which causes issues further down the line in apply processing. Regards, Nikhils > Greetings, > > Andres Freund -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: Log query parameters for terminated execute
Hello!
>> After shutdown signal we must be in aborted state, so we mustn't call
>> user-defined I/O functions. (quotation from comment to errdetail_params in
>> src/backend/tcop/postgres.c ). It seems i can not fix it with current design.
>
> No its not about calling the function after abort/shutdown. Just start the
> server and try to run the program, most of the time you will not get params.
>
>>> ERROR: canceling statement due to lock timeout at character 13
>> Hm, "at character"? How can we get this message? I found only "canceling
>> statement due to lock timeout" (without "at character") ereport in
>> src/backend/tcop/postgres.c
>> Maybe try .. catch in parse state, not in execute?
>
> Its really easy to reproduce, just lock the table form another session and
> run a "c" program to insert row in the same table.
So, I was right. We can got "canceling statement due to lock timeout at
character 13" only in PARSE state. In parse state we have completely no
parameters, we receive parameters only later in BIND message. I can not log
data from future.
And initially i did change only EXECUTE. Attached patch with similar change in
BIND message, if this design is acceptable.
Please test with logging command tag %i in log_line_prefix. Extended protocol
has three different messages, each can be canceled by timeout. But here is
completely no parameters in PARSE and i did not change BIND in first patch.
regards, Sergeidiff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index f413395..c6a7cb2 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -171,6 +171,9 @@ static ProcSignalReason RecoveryConflictReason;
static MemoryContext row_description_context = NULL;
static StringInfoData row_description_buf;
+/* current portal parameters */
+static ParamListInfo debug_query_params = NULL;
+
/*
* decls for routines only used in this file
*
@@ -183,7 +186,7 @@ static void forbidden_in_wal_sender(char firstchar);
static List *pg_rewrite_query(Query *query);
static bool check_log_statement(List *stmt_list);
static int errdetail_execute(List *raw_parsetree_list);
-static int errdetail_params(ParamListInfo params);
+static int errdetail_log_params(ParamListInfo params);
static int errdetail_abort(void);
static int errdetail_recovery_conflict(void);
static void start_xact_command(void);
@@ -1771,6 +1774,7 @@ exec_bind_message(StringInfo input_message)
params->params[paramno].pflags = PARAM_FLAG_CONST;
params->params[paramno].ptype = ptype;
}
+ debug_query_params = params;
}
else
params = NULL;
@@ -1850,7 +1854,7 @@ exec_bind_message(StringInfo input_message)
*portal_name ? portal_name : "",
psrc->query_string),
errhidestmt(true),
- errdetail_params(params)));
+ errdetail_log_params(params)));
break;
}
@@ -1858,6 +1862,7 @@ exec_bind_message(StringInfo input_message)
ShowUsage("BIND MESSAGE STATISTICS");
debug_query_string = NULL;
+ debug_query_params = NULL;
}
/*
@@ -1934,6 +1939,7 @@ exec_execute_message(const char *portal_name, long max_rows)
else
prepStmtName = "";
portalParams = portal->portalParams;
+ debug_query_params = portalParams;
}
/*
@@ -1985,7 +1991,7 @@ exec_execute_message(const char *portal_name, long max_rows)
*portal_name ? portal_name : "",
sourceText),
errhidestmt(true),
- errdetail_params(portalParams)));
+ errdetail_log_params(portalParams)));
was_logged = true;
}
@@ -2074,7 +2080,7 @@ exec_execute_message(const char *portal_name, long max_rows)
*portal_name ? portal_name : "",
sourceText),
errhidestmt(true),
- errdetail_params(portalParams)));
+ errdetail_log_params(portalParams)));
break;
}
@@ -2082,6 +2088,7 @@ exec_execute_message(const char *portal_name, long max_rows)
ShowUsage("EXECUTE MESSAGE STATISTICS");
debug_query_string = NULL;
+ debug_query_params = NULL;
}
/*
@@ -2200,12 +2207,12 @@ errdetail_execute(List *raw_parsetree_list)
}
/*
- * errdetail_params
+ * errdetail_log_params
*
- * Add an errdetail() line showing bind-parameter data, if available.
+ * Add an errdetail_log() line showing bind-parameter data, if available.
*/
static int
-errdetail_params(ParamListInfo params)
+errdetail_log_params(ParamListInfo params)
{
/* We mustn't call user-defined I/O functions when in an aborted xact */
if (params && params->numParams > 0 && !IsAbortedTransactionBlockState())
@@ -2256,7 +2263,7 @@ errdetail_params(ParamListInfo params)
pfree(pstring);
}
- errdetail("parameters: %s", param_str.data);
+ errdetail_log("parameters: %s", param_str.data);
pfree(param_str.data);
@@ -2925,7 +2932,8 @@ ProcessInterrupts(void)
else
ereport(FATAL,
(errcode(ERRCODE_ADMIN_SHUTDOWN),
- errmsg("
Re: [bug fix] Produce a crash dump before main() on Windows
On Mon, Jul 23, 2018 at 12:56 PM, Michael Paquier wrote: > On Mon, Jul 23, 2018 at 01:31:57AM +, Tsunakawa, Takayuki wrote: >> First, as Hari-san said, starting the server with "pg_ctl start" on >> the command prompt does not feel like production use but like test >> environment, and that usage seems mostly interactive. Second, the >> current PostgreSQL is not exempt from the same problem: if postmaster >> or standalone backend crashes before main(), the pop up would appear. > > When you use pg_ctl start within the command prompt, then closing the > prompt session also causes Postgres to stop immediately. Would it be a > problem? > > Another question I have is that I have seen Postgres fail to start > because of missing dependent libraries, which happened after the > postmaster startup as this was reported in the logs, could it be a > problem with this patch? > It doesn't appear so, because we have SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX); in dlopen. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: ToDo: show size of partitioned table
Hi
I am sending a prototype of patch. Now, it calculates size of partitioned
tables with recursive query. When any more simple method will be possible,
the size calculation will be changed.
postgres=# \dt+
List of relations
+++---+---+-+-+
| Schema |Name| Type | Owner | Size | Description |
+++---+---+-+-+
| public | data | table | pavel | 0 bytes | |
| public | data_2016 | table | pavel | 15 MB | |
| public | data_2017 | table | pavel | 15 MB | |
| public | data_other | table | pavel | 11 MB | |
+++---+---+-+-+
(4 rows)
postgres=# \dP+
List of partitioned tables
++--+---+---+-+
| Schema | Name | Owner | Size | Description |
++--+---+---+-+
| public | data | pavel | 42 MB | |
++--+---+---+-+
(1 row)
Regards
Pavel
p.s. Another patch can be replacement of relation type from "table" to
"partitioned table"
postgres=# \dt+
List of relations
+++---+---+-+-+
| Schema |Name| Type| Owner | Size | Description |
+++---+---+-+-+
| public | data | partitioned table | pavel | 0 bytes | |
| public | data_2016 | table | pavel | 15 MB | |
| public | data_2017 | table | pavel | 15 MB | |
| public | data_other | table | pavel | 11 MB | |
+++---+---+-+-+
(4 rows)
A patch is simple for this case
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index c3bdf8555d..491e58eb29 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3490,8 +3490,8 @@ listTables(const char *tabtypes, const char *pattern,
bool verbose, bool showSys
gettext_noop("sequence"),
gettext_noop("special"),
gettext_noop("foreign table"),
- gettext_noop("table"),/* partitioned table */
- gettext_noop("index"),/* partitioned index */
+ gettext_noop("partitioned table"),/* partitioned
table */
+ gettext_noop("partitioned index"),/* partitioned
index */
gettext_noop("Type"),
gettext_noop("Owner"));
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b17039d60f..d2eb701a96 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1635,6 +1635,21 @@ testdb=>
+
+
+\dP[+] [ pattern ]
+
+
+Lists partitioned tables. If pattern is
+specified, only entries whose table name or schema name matches
+the pattern are listed. If the form \dP+
+is used, a sum of size of related partitions and a description
+are also displayed.
+
+
+
+
+
\drds [ role-pattern [ database-pattern ] ]
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 4c85f43f09..09f49b4f39 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -785,6 +785,9 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
case 'p':
success = permissionsList(pattern);
break;
+ case 'P':
+success = listPartitions(pattern, show_verbose);
+break;
case 'T':
success = describeTypes(pattern, show_verbose, show_system);
break;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index c3bdf8555d..4542fd511c 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3601,6 +3601,122 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
return true;
}
+/*
+ * listPartitions()
+ *
+ * handler for \dP
+ */
+bool
+listPartitions(const char *pattern, bool verbose)
+{
+ PQExpBufferData buf;
+ PGresult *res;
+ printQueryOpt myopt = pset.popt;
+ static const bool translate_columns[] = {false, false, true, false, false, false, false};
+
+ initPQExpBuffer(&buf);
+
+ /*
+ * Note: as of Pg 8.2, we no longer use relkind 's' (special), but we keep
+ * it here for backwards compatibility.
+ */
+ printfPQExpBuffer(&buf,
+ "SELECT n.nspname as \"%s\",\n"
+ " c.relname as \"%s\",\n"
+ " pg_catalog.pg_get_userbyid(c.relowner) as \"%s\"",
+ gettext_noop("Schema"),
+ gettext_noop("Name"),
+ gettext_noop("Owner"));
+
+ if (verbose)
+ {
+ /* CTE is supported from 8.4 */
+ if (pset.sversion >= 80400)
+ {
+ appendPQExpBuffer(&buf,
+ ",\n (WITH R
Re: pgbench: improve --help and --version parsing
Hello Michaël,
I doubt that -V & -? are heavily tested:-) Patch works for me, though.
They are not, and the patch misses this area.
Indeed.
I don't think that it is a bad idea to improve things the way you are
For the record, this is not my patch, I'm merely reviewing it.
doing, however you should extend program_version_ok() and
program_help_ok() in src/test/perl/TestLib.pm so as short options are
tested for two reasons:
Interesting, I did not notice these functions before. I fully agree that
manual testing is a pain for such a simple change.
Do you mean something like the attached?
--
Fabien.diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl
b/src/bin/pgbench/t/002_pgbench_no_server.pl
index a2845a583b..24ecfec33e 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -56,6 +56,10 @@ sub pgbench_scripts
return;
}
+# help and version sanity checks
+program_help_ok("pgbench");
+program_version_ok("pgbench");
+
#
# Option various errors
#
@@ -205,7 +209,7 @@ pgbench(
'pgbench help');
# Version
-pgbench('-V', 0, [qr{^pgbench .PostgreSQL. }], [qr{^$}], 'pgbench version');
+pgbench('-V', 0, [qr{^pgbench \(PostgreSQL\) \d+}], [qr{^$}], 'pgbench
version');
# list of builtins
pgbench(
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 3a184a4fad..7799157335 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -409,13 +409,25 @@ sub program_help_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
+ my ($stdout, $stdout2, $stderr, $stderr2);
print("# Running: $cmd --help\n");
my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
\$stderr;
+ my $result2 = IPC::Run::run [ $cmd, '-?' ], '>', \$stdout2, '2>',
+ \$stderr2;
ok($result, "$cmd --help exit code 0");
+ ok($result2, "$cmd -? exit code 0");
isnt($stdout, '', "$cmd --help goes to stdout");
+ is($stdout, $stdout2, "$cmd --help and -? have same output");
+ like($stdout, qr{Usage:}, "$cmd --help is about usage");
+ like($stdout, qr{\b$cmd\b}, "$cmd --help is about $cmd");
+ like($stdout, qr{--help}, "$cmd --help is about option --help");
+ like($stdout, qr{-\?}, "$cmd --help is about options -?");
+ like($stdout, qr{--version}, "$cmd --help is about options --version");
+ like($stdout, qr{-V}, "$cmd --help is about options -V");
+ # more sanity check on the output? eg Report bug to ..., options:...
is($stderr, '', "$cmd --help nothing to stderr");
+ is($stderr2, '', "$cmd -? nothing to stderr");
return;
}
@@ -423,13 +435,19 @@ sub program_version_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
+ my ($stdout, $stdout2, $stderr, $stderr2);
print("# Running: $cmd --version\n");
my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
\$stderr;
+ my $result2 = IPC::Run::run [ $cmd, '-V' ], '>', \$stdout2, '2>',
+ \$stderr2;
ok($result, "$cmd --version exit code 0");
+ ok($result2, "$cmd -V exit code 0");
isnt($stdout, '', "$cmd --version goes to stdout");
+ is($stdout, $stdout2, "$cmd --version and -V have same output");
+ like($stdout, qr{^$cmd \(PostgreSQL\) \d+(devel|\.\d+)\b}, "$cmd
--version looks like a version");
is($stderr, '', "$cmd --version nothing to stderr");
+ is($stderr2, '', "$cmd -V nothing to stderr");
return;
}
Re: [HACKERS] Two pass CheckDeadlock in contentent case
On 3 October 2017 at 15:30, Sokolov Yura wrote: > If hundreds of backends reaches this timeout trying to acquire > advisory lock on a same value, it leads to hard-stuck for many > seconds, cause they all traverse same huge lock graph under > exclusive lock. > During this stuck there is no possibility to do any meaningful > operations (no new transaction can begin). Well observed, we clearly need to improve this. > Attached patch makes CheckDeadlock to do two passes: > - first pass uses LW_SHARED on partitions of lock hash. > DeadLockCheck is called with flag "readonly", so it doesn't > modify anything. > - If there is possibility of "soft" or "hard" deadlock detected, > ie if there is need to modify lock graph, then partitions > relocked with LW_EXCLUSIVE, and DeadLockCheck is called again. > > It fixes hard-stuck, cause backends walk lock graph under shared > lock, and found that there is no real deadlock. In phase 2, does this relock only the partitions required to reorder the lock graph, or does it request all locks? Fewer locks would be better. If you decide to reorder the lock graph, then only one backend should attempt this at a time. We should keep track of reorder-requests, so if two backends arrive at the same conclusion then only one should proceed to do this. Many deadlocks happen between locks in same table. It would also be a useful optimization to check just one partition for lock graphs before we checked all partitions. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re[2]: Alter index rename concurrently to
>Среда, 18 июля 2018, 12:21 +03:00 от Peter Eisentraut >: > > >In your patch, the effect of the CONCURRENTLY keyword is just to change >the lock level, without any further changes. That doesn't make much >sense. If we think the lower lock level is OK, then we should just use >it always. I was thinking about it and I seem that AccessShareExclusiveLock by default is a better way than the lower lock level because it corresponds to the principle of more strict implementation of changing database schema (by default). I think if some users need to rename a relation without query locking they should say it definitely by using "concurrently" keyword like in the drop/create index cases. Otherwise, to set the lower lock level we must also embody new keyword for "alter.. rename" to allow user to set up stronger lock level for this action (not only indexes but tables and so on). Moreover, if you rename Table without query locking, it may crushes your services that do queries at the same time, therefore , this is unlikely that someone will be do it with concurrent queries to renamed table, in other words, with running production. So, I think it doesn't have real sense to use the lower lock for example for tables (at least by default). However, renaming Indexes with the lower lock is safe for database consumers because they don't know anything about them. As I wrote early, in the patch I added the 'concurrent' field in the RenameStmt structure. However, the "concurrently" keyword is realized there for index renaming only because I only see need to rename indexes in concurrent mode. It also may be implemented for tables, etc if it will be really needed for someone in the future. -- Kind regards, Andrey Klychkov
Re: [HACKERS] Custom compression methods
Hi! On Mon, Jul 2, 2018 at 3:56 PM Ildus Kurbangaliev wrote: > On Mon, 18 Jun 2018 17:30:45 +0300 > Ildus Kurbangaliev wrote: > > > On Tue, 24 Apr 2018 14:05:20 +0300 > > Alexander Korotkov wrote: > > > > > > > > Yes, this patch definitely lacks of good usage example. That may > > > lead to some misunderstanding of its purpose. Good use-cases > > > should be shown before we can consider committing this. I think > > > Ildus should try to implement at least custom dictionary compression > > > method where dictionary is specified by user in parameters. > > > > > > > Hi, > > > > attached v16 of the patch. I have splitted the patch to 8 parts so now > > it should be easier to make a review. The main improvement is zlib > > compression method with dictionary support like you mentioned. My > > synthetic tests showed that zlib gives more compression but usually > > slower than pglz. > > > > I have noticed that my patch is failing to apply on cputube. Attached a > rebased version of the patch. Nothing have really changed, just added > and fixed some tests for zlib and improved documentation. I'm going to review this patch. Could you please rebase it? It doesn't apply for me due to changes made in src/bin/psql/describe.c. patching file src/bin/psql/describe.c Hunk #1 FAILED at 1755. Hunk #2 FAILED at 1887. Hunk #3 FAILED at 1989. Hunk #4 FAILED at 2019. Hunk #5 FAILED at 2030. 5 out of 5 hunks FAILED -- saving rejects to file src/bin/psql/describe.c.rej Also, please not that PostgreSQL 11 already passed feature freeze some time ago. So, please adjust your patch to expect PostgreSQL 12 in the lines like this: + if (pset.sversion >= 11) -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: project updates
Hello Charles, > Having tried David's method to install 10.4 and 11 on my mac and > turns out worked for me. The compiling issue posted by Aleksander is > because some json helpers changed function name and is not backward > compatible with 9.4 and 10. Using #if macro resolves the problem, > Here is the commit to solve this. > https://github.com/charles-cui/pg_thrift/commit/dd5b8ad17ab47e3c977943dd2d69e5abad34c6ad > Aleksander, do you want to test again? It's much better now, thank you! :) -- Best regards, Aleksander Alekseev
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 9, 2018 at 08:29:08AM +, Tsunakawa, Takayuki wrote: > Thank you for supporting me, Andres. And please don't mind, David. I > don't think you are attacking me. I understand your concern and that > you are also trying to protect PostgreSQL. > > On the other hand, I think TPL seems less defensive. I read > in some report that Apache License and some other open source > licenses were created partly due to lack of patent description > in BSD and GPLv2. > > How can we assure you? How about attaching something like the > following to relevant patches or on our web site? > > [Excerpt from Red Hat Patent Promise] Red Hat intends Our Promise to > be irrevocable (except as stated herein), and binding and enforceable > against Red Hat and assignees of, or successors to, Red Hat’s > patents (and any patents directly or indirectly issuing from Red > Hat’s patent applications). As part of Our Promise, if Red Hat > sells, exclusively licenses, or otherwise assigns or transfers > patents or patent applications to a party, we will require the party > to agree in writing to be bound to Our Promise for those patents > and for patents directly or indirectly issuing on those patent > applications. We will also require the party to agree in writing to so > bind its own assignees, transferees, and exclusive licensees. Notice this makes no mention of what happens to the patents if the company goes bankrupt. My guess is that in such a situation the company would have no control over who buys the patents or how they are used. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: [HACKERS] Bug in to_timestamp().
On Sun, Jul 22, 2018 at 08:22:23AM -0700, David G. Johnston wrote:
> My re-read of the thread the other day left me with a feeling of
> contentment that this was an acceptable change but I also get the feeling
> like I'm missing the downside trade-off too...I was hoping your review
> would help in that regard but as it did not speak to specific
> incompatibilities it has not.
I like more behaviour of the function with the patch. It gives less
unexpected results. For example, the query mentioned above:
SELECT to_timestamp('2011-12-18 23:38:15', '-MM-DD HH24:MI:SS')
I looked for some tradeoffs of the patch. I think it could be parsing
strings like the following input strings:
SELECT TO_TIMESTAMP('2011年5月1日', '-MM-DD');
SELECT TO_TIMESTAMP('2011y5m1d', '-MM-DD');
HEAD extracts year, month and day from the string. But patched
to_timestamp() raises an error. Someone could rely on such behaviour.
The patch divides separator characters from letters and digits. And
'年' or 'y' are letters here. And so the format string doesn't match the
input string.
--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Re: How can we submit code patches that implement our (pending) patents?
On July 23, 2018 6:25:42 AM PDT, Bruce Momjian wrote: >On Mon, Jul 9, 2018 at 08:29:08AM +, Tsunakawa, Takayuki wrote: >> Thank you for supporting me, Andres. And please don't mind, David. >I >> don't think you are attacking me. I understand your concern and that >> you are also trying to protect PostgreSQL. >> >> On the other hand, I think TPL seems less defensive. I read >> in some report that Apache License and some other open source >> licenses were created partly due to lack of patent description >> in BSD and GPLv2. >> >> How can we assure you? How about attaching something like the >> following to relevant patches or on our web site? >> >> [Excerpt from Red Hat Patent Promise] Red Hat intends Our Promise to >> be irrevocable (except as stated herein), and binding and enforceable >> against Red Hat and assignees of, or successors to, Red Hat’s >> patents (and any patents directly or indirectly issuing from Red >> Hat’s patent applications). As part of Our Promise, if Red Hat >> sells, exclusively licenses, or otherwise assigns or transfers >> patents or patent applications to a party, we will require the party >> to agree in writing to be bound to Our Promise for those patents >> and for patents directly or indirectly issuing on those patent >> applications. We will also require the party to agree in writing to >so >> bind its own assignees, transferees, and exclusive licensees. > >Notice this makes no mention of what happens to the patents if the >company goes bankrupt. My guess is that in such a situation the >company >would have no control over who buys the patents or how they are used. It explicitly says irrevocable and successors. Why seems squarely aimed at your concern. Bankruptcy wouldn't just invalidate that. Similarly icenses like Apache 2 grant a perpetual and irrevocable patent grant for the use in the contribution. Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: [HACKERS] logical decoding of two-phase transactions
Hi,
On 2018-07-23 16:31:50 +0530, Nikhil Sontakke wrote:
> >> > That'd require that an index lookup can't crash if the corresponding
> >> > heap entry doesn't exist (etc), but that's something we need to handle
> >> > anyway. The issue that multiple separate catalog lookups need to be
> >> > coherent (say Robert's pg_class exists, but pg_attribute doesn't
> >> > example) is solved by virtue of the the pg_attribute lookups failing if
> >> > the transaction aborted.
> > Not quite, no. Basically, in a simplified manner, the logical decoding
> > loop is like:
> >
> > while (true)
> > record = readRecord()
> > logical = decodeRecord()
> >
> > PG_TRY():
> > StartTransactionCommand();
> >
> > switch (TypeOf(logical))
> > case INSERT:
> > insert_callback(logical);
> > break;
> > ...
> >
> > CommitTransactionCommand();
> >
> > PG_CATCH():
> > AbortCurrentTransaction();
> > PG_RE_THROW();
> >
> > what I'm proposing is that that various catalog access functions throw a
> > new class of error, something like "decoding aborted transactions".
>
> When will this error be thrown by the catalog functions? How will it
> determine that it needs to throw this error?
The error check would have to happen at the end of most systable_*
functions. They'd simply do something like
if (decoding_in_progress_xact && TransactionIdDidAbort(xid_of_aborted))
ereport(ERROR, (errcode(DECODING_ABORTED_XACT), errmsg("oops")));
i.e. check whether the transaction to be decoded still is in
progress. As that would happen before any potentially wrong result can
be returned (as the check happens at the tail end of systable_*),
there's no issue with wrong state in the syscache etc.
> > PG_CATCH():
> > AbortCurrentTransaction();
> > if (errclass == DECODING_ABORTED_XACT)
> >in_progress_xact_abort_pending = true;
> >continue;
> > else
> >PG_RE_THROW();
> >
> > Now obviously that's just pseudo code with lotsa things missing, but I
> > think the basic idea should come through?
> >
>
> How do we handle the cases where the catalog returns inconsistent data
> (without erroring out) which does not help with the ongoing decoding?
> Consider for example:
I don't think that situation exists, given the scheme described
above. That's just the point.
> BEGIN;
> /* CONSIDER T1 has one column C1 */
> ALTER TABLE T1 ADD COL c2;
> INSERT INTO TABLE T1(c2) VALUES;
> PREPARE TRANSACTION;
>
> If we abort the above 2PC and the catalog row for the ALTER gets
> cleaned up by vacuum, then the catalog read will return us T1 with one
> column C1.
No, it'd throw an error due to the bew is-aborted check.
> The catalog scan will NOT error out but will return metadata which
> causes the insert-decoding change apply callback to error out.
Why would it not throw an error?
Greetings,
Andres Freund
Re: How can we submit code patches that implement our (pending) patents?
On Tue, Jul 10, 2018 at 09:47:09AM -0400, Tom Lane wrote: > "Tsunakawa, Takayuki" writes: > > From: Nico Williams [mailto:n...@cryptonector.com] > >> ... But that might reduce the > >> size of the community, or lead to a fork. > > > Yes, that's one unfortunate future, which I don't want to happen of > > course. I believe PostgreSQL should accept patent for further > > evolution, because PostgreSQL is now a popular, influential software > > that many organizations want to join. > > The core team has considered this matter, and has concluded that it's > time to establish a firm project policy that we will not accept any code > that is known to be patent-encumbered. The long-term legal risks and > complications involved in doing that seem insurmountable, given the > community's amorphous legal nature and the existing Postgres license > wording (neither of which are open for negotiation here). Furthermore, > Postgres has always been very friendly to creation of closed-source > derivatives, but it's hard to see how inclusion of patented code would > not cause serious problems for those. The potential benefits of > accepting patented code just don't seem to justify trying to navigate > these hazards. Just to add a summary to this, any patent assignment to Postgres would have to allow free patent use for all code, under _any_ license. This effectively makes the patent useless, except for defensive use, even for the patent owner. I think everyone here agrees on this. The open question is whether it is useful for the PGDG to accept such patents for defensive use. There are practical problems with this (PGDG is not a legal entity) and operational complexity too. The core team's feeling is that it not worth it, but that discussion can be re-litigated on this email list if desired. The discussion would have to relate to such patents in general, not to the specific Fujitsu proposal. If it was determined that such defensive patents were desired, we can then consider the Fujitsu proposal. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 06:31:14AM -0700, Andres Freund wrote: > On July 23, 2018 6:25:42 AM PDT, Bruce Momjian wrote: > >On Mon, Jul 9, 2018 at 08:29:08AM +, Tsunakawa, Takayuki wrote: > >> Thank you for supporting me, Andres. And please don't mind, David. > >I > >> don't think you are attacking me. I understand your concern and that > >> you are also trying to protect PostgreSQL. > >> > >> On the other hand, I think TPL seems less defensive. I read > >> in some report that Apache License and some other open source > >> licenses were created partly due to lack of patent description > >> in BSD and GPLv2. > >> > >> How can we assure you? How about attaching something like the > >> following to relevant patches or on our web site? > >> > >> [Excerpt from Red Hat Patent Promise] Red Hat intends Our Promise to > >> be irrevocable (except as stated herein), and binding and enforceable > >> against Red Hat and assignees of, or successors to, Red Hat’s > >> patents (and any patents directly or indirectly issuing from Red > >> Hat’s patent applications). As part of Our Promise, if Red Hat > >> sells, exclusively licenses, or otherwise assigns or transfers > >> patents or patent applications to a party, we will require the party > >> to agree in writing to be bound to Our Promise for those patents > >> and for patents directly or indirectly issuing on those patent > >> applications. We will also require the party to agree in writing to > >so > >> bind its own assignees, transferees, and exclusive licensees. > > > >Notice this makes no mention of what happens to the patents if the > >company goes bankrupt. My guess is that in such a situation the > >company > >would have no control over who buys the patents or how they are used. > > It explicitly says irrevocable and successors. Why seems squarely aimed at > your concern. Bankruptcy wouldn't just invalidate that. They can say whatever they want, but if they are bankrupt, what they say doesn't matter much. My guess is that they would have to give their patents to some legal entity that owns them so it is shielded from bankrupcy. > Similarly icenses like Apache 2 grant a perpetual and irrevocable patent > grant for the use in the contribution. As far as I know, this is exactly that case that the company is giving irrevocable patent access to an external entity, but only for Apache-licensed code. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: [Proposal] Add accumulated statistics for wait event
Michael Paquier writes: > This does not need a configure switch. It probably is there because the OP realizes that most people wouldn't accept having this code compiled in. > What's the performance penalty? I am pretty sure that this is > measurable as wait events are stored for a backend for each I/O > operation as well, and you are calling a C routine within an inlined > function which is designed to be light-weight, doing only a four-byte > atomic operation. On machines with slow gettimeofday(), I suspect the cost of this patch would be staggering. Even with relatively fast gettimeofday, it doesn't look acceptable for calls in hot code paths (for instance, lwlock.c). A bigger problem is that it breaks stuff. There are countless calls to pgstat_report_wait_start/pgstat_report_wait_end that assume they have no side-effects (for example, on errno) and can never fail. I wouldn't trust GetCurrentTimestamp() for either. If the report_wait calls can't be dropped into code with *complete* certainty that they're safe, that's a big cost. Why exactly is this insisting on logging timestamps and not, say, just incrementing a counter? I think doing it like this is almost certain to end in rejection. regards, tom lane
Re: How can we submit code patches that implement our (pending) patents?
On Tue, Jul 10, 2018 at 08:20:53AM +, Tsunakawa, Takayuki wrote: > > One possible answer is that you wouldn't. But that might reduce the > > size of the community, or lead to a fork. > > Yes, that's one unfortunate future, which I don't want to happen > of course. I believe PostgreSQL should accept patent for further > evolution, because PostgreSQL is now a popular, influential software > that many organizations want to join. Why did you say this last sentence? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 09:53:26AM -0400, Bruce Momjian wrote: > On Tue, Jul 10, 2018 at 09:47:09AM -0400, Tom Lane wrote: > > The core team has considered this matter, and has concluded that it's > > time to establish a firm project policy that we will not accept any code > > that is known to be patent-encumbered. The long-term legal risks and > > complications involved in doing that seem insurmountable, given the > > community's amorphous legal nature and the existing Postgres license > > wording (neither of which are open for negotiation here). Furthermore, > > Postgres has always been very friendly to creation of closed-source > > derivatives, but it's hard to see how inclusion of patented code would > > not cause serious problems for those. The potential benefits of > > accepting patented code just don't seem to justify trying to navigate > > these hazards. > > Just to add a summary to this, any patent assignment to Postgres would > have to allow free patent use for all code, under _any_ license. This > effectively makes the patent useless, except for defensive use, even for > the patent owner. I think everyone here agrees on this. > > The open question is whether it is useful for the PGDG to accept such > patents for defensive use. There are practical problems with this (PGDG > is not a legal entity) and operational complexity too. The core team's > feeling is that it not worth it, but that discussion can be re-litigated > on this email list if desired. The discussion would have to relate to > such patents in general, not to the specific Fujitsu proposal. If it > was determined that such defensive patents were desired, we can then > consider the Fujitsu proposal. And the larger question is whether a patent free for use by software under any license can be used in a defensive way. If not, it means we have no way forward here. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: Log query parameters for terminated execute
Sergei Kornilov writes: > Please test with logging command tag %i in log_line_prefix. Extended protocol > has three different messages, each can be canceled by timeout. But here is > completely no parameters in PARSE and i did not change BIND in first patch. This patch scares me to death. It risks calling user-defined I/O functions in all sorts of weird places, particularly outside transactions, or in already-failed transactions, or with no ActiveSnapshot. Separately from that concern: it appears to result in a substantial degradation of existing functionality in the places where you did s/errdetail/errdetail_log/. What was the reason for that? regards, tom lane
Re: [HACKERS] logical decoding of two-phase transactions
Hi Andres,
>> > what I'm proposing is that that various catalog access functions throw a
>> > new class of error, something like "decoding aborted transactions".
>>
>> When will this error be thrown by the catalog functions? How will it
>> determine that it needs to throw this error?
>
> The error check would have to happen at the end of most systable_*
> functions. They'd simply do something like
>
> if (decoding_in_progress_xact && TransactionIdDidAbort(xid_of_aborted))
>ereport(ERROR, (errcode(DECODING_ABORTED_XACT), errmsg("oops")));
>
> i.e. check whether the transaction to be decoded still is in
> progress. As that would happen before any potentially wrong result can
> be returned (as the check happens at the tail end of systable_*),
> there's no issue with wrong state in the syscache etc.
>
Oh, ok. The systable_* functions use the passed in snapshot and return
tuples matching to it. They do not typically have access to the
current XID being worked upon..
We can find out if the snapshot is a logical decoding one by virtue of
its "satisfies" function pointing to HeapTupleSatisfiesHistoricMVCC.
>
>> The catalog scan will NOT error out but will return metadata which
>> causes the insert-decoding change apply callback to error out.
>
> Why would it not throw an error?
>
In your scheme, it will throw an error, indeed. We'd need to make the
"being-currently-decoded-XID" visible to these systable_* functions
and then this scheme will work.
Regards,
Nikhils
> Greetings,
>
> Andres Freund
--
Nikhil Sontakke http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Re: How can we submit code patches that implement our (pending) patents?
On 2018-07-23 09:56:47 -0400, Bruce Momjian wrote: > On Mon, Jul 23, 2018 at 06:31:14AM -0700, Andres Freund wrote: > > On July 23, 2018 6:25:42 AM PDT, Bruce Momjian wrote: > > >On Mon, Jul 9, 2018 at 08:29:08AM +, Tsunakawa, Takayuki wrote: > > >> Thank you for supporting me, Andres. And please don't mind, David. > > >I > > >> don't think you are attacking me. I understand your concern and that > > >> you are also trying to protect PostgreSQL. > > >> > > >> On the other hand, I think TPL seems less defensive. I read > > >> in some report that Apache License and some other open source > > >> licenses were created partly due to lack of patent description > > >> in BSD and GPLv2. > > >> > > >> How can we assure you? How about attaching something like the > > >> following to relevant patches or on our web site? > > >> > > >> [Excerpt from Red Hat Patent Promise] Red Hat intends Our Promise to > > >> be irrevocable (except as stated herein), and binding and enforceable > > >> against Red Hat and assignees of, or successors to, Red Hat’s > > >> patents (and any patents directly or indirectly issuing from Red > > >> Hat’s patent applications). As part of Our Promise, if Red Hat > > >> sells, exclusively licenses, or otherwise assigns or transfers > > >> patents or patent applications to a party, we will require the party > > >> to agree in writing to be bound to Our Promise for those patents > > >> and for patents directly or indirectly issuing on those patent > > >> applications. We will also require the party to agree in writing to > > >so > > >> bind its own assignees, transferees, and exclusive licensees. > > > > > >Notice this makes no mention of what happens to the patents if the > > >company goes bankrupt. My guess is that in such a situation the > > >company > > >would have no control over who buys the patents or how they are used. > > > > It explicitly says irrevocable and successors. Why seems squarely aimed at > > your concern. Bankruptcy wouldn't just invalidate that. > > They can say whatever they want, but if they are bankrupt, what they say > doesn't matter much. My guess is that they would have to give their > patents to some legal entity that owns them so it is shielded from > bankrupcy. Huh? Bancruptcy doesn't simply invalidate licenses which successors then can ignore. By that logic a license to use the code, like the PG license, would be just as ineffectual Greetings, Andres Freund
Re: [HACKERS] Bug in to_timestamp().
On Mon, Jul 23, 2018 at 04:30:43PM +0300, Arthur Zakirov wrote:
> I looked for some tradeoffs of the patch. I think it could be parsing
> strings like the following input strings:
>
> SELECT TO_TIMESTAMP('2011年5月1日', '-MM-DD');
> SELECT TO_TIMESTAMP('2011y5m1d', '-MM-DD');
>
> HEAD extracts year, month and day from the string. But patched
> to_timestamp() raises an error. Someone could rely on such behaviour.
> The patch divides separator characters from letters and digits. And
> '年' or 'y' are letters here. And so the format string doesn't match the
> input string.
Sorry, I forgot to mention that the patch can handle this by using
different format string. You can execute:
=# SELECT TO_TIMESTAMP('2011年5月1日', '年MM月DD日');
to_timestamp
2011-05-01 00:00:00+04
=# SELECT TO_TIMESTAMP('2011y5m1d', '"y"MM"m"DD"d"');
to_timestamp
2011-05-01 00:00:00+04
or:
=# SELECT TO_TIMESTAMP('2011y5m1d', 'tMMtDDt');
to_timestamp
2011-05-01 00:00:00+04
--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Re: How can we submit code patches that implement our (pending) patents?
On 07/23/2018 10:01 AM, Bruce Momjian wrote: > And the larger question is whether a patent free for use by software > under any license can be used in a defensive way. If not, it means we > have no way forward here. Isn't 'defensive', in patent-speak, used to mean 'establishing prior art usable to challenge future patent claims by others on the same technique'? Is there any way that conditions of use, or lack of them, on an existing patent, would make it unusable in that context? -Chap
Re: cached plans and enable_partition_pruning
Hi, On 2018-07-23 18:31:43 +0900, Amit Langote wrote: > It seems that because enable_partition_pruning's value is only checked > during planning, turning it off *after* a plan is created and cached does > not work as expected. > > create table p (a int) partition by list (a); > create table p1 partition of p for values in (1); > create table p1 partition of p for values in (2); > > -- force a generic plan so that run-time pruning is used in the plan > reset enable_partition_pruning; > set plan_cache_mode to force_generic_plan; > prepare p as select * from p where a = $1; > > explain (costs off, analyze) execute p (1); >QUERY PLAN > > Append (actual time=0.079..0.106 rows=1 loops=1) >Subplans Removed: 1 >-> Seq Scan on p2 (actual time=0.058..0.068 rows=1 loops=1) > Filter: (a = $1) > Planning Time: 17.573 ms > Execution Time: 0.396 ms > (6 rows) > > set enable_partition_pruning to off; > > explain (costs off, analyze) execute p (1); >QUERY PLAN > > Append (actual time=0.108..0.135 rows=1 loops=1) >Subplans Removed: 1 >-> Seq Scan on p2 (actual time=0.017..0.028 rows=1 loops=1) > Filter: (a = $1) > Planning Time: 0.042 ms > Execution Time: 0.399 ms > (6 rows) > > Pruning still occurs, whereas one would expect it not to, because the plan > (the Append node) contains run-time pruning information, which was > initialized because enable_partition_pruning was turned on when the plan > was created. > > Should we check its value during execution too, as done in the attached? I think it's correct to check the plan time value, rather than the execution time value. Other enable_* GUCs also take effect there, and I don't see a problem with that? Greetings, Andres Freund
Re: [HACKERS] Bug in to_timestamp().
On Mon, Jul 23, 2018 at 5:12 PM Arthur Zakirov wrote:
> On Mon, Jul 23, 2018 at 04:30:43PM +0300, Arthur Zakirov wrote:
> > I looked for some tradeoffs of the patch. I think it could be parsing
> > strings like the following input strings:
> >
> > SELECT TO_TIMESTAMP('2011年5月1日', '-MM-DD');
> > SELECT TO_TIMESTAMP('2011y5m1d', '-MM-DD');
> >
> > HEAD extracts year, month and day from the string. But patched
> > to_timestamp() raises an error. Someone could rely on such behaviour.
> > The patch divides separator characters from letters and digits. And
> > '年' or 'y' are letters here. And so the format string doesn't match the
> > input string.
>
> Sorry, I forgot to mention that the patch can handle this by using
> different format string. You can execute:
>
> =# SELECT TO_TIMESTAMP('2011年5月1日', '年MM月DD日');
> to_timestamp
>
> 2011-05-01 00:00:00+04
>
> =# SELECT TO_TIMESTAMP('2011y5m1d', '"y"MM"m"DD"d"');
> to_timestamp
>
> 2011-05-01 00:00:00+04
>
> or:
>
> =# SELECT TO_TIMESTAMP('2011y5m1d', 'tMMtDDt');
> to_timestamp
>
> 2011-05-01 00:00:00+04
Thank you, Arthur. These examples shows downside of this patch, where
users may be faced with incompatibility. But it's good that this
situation can be handled by altering format string. I think these
examples should be added to the documentation and highlighted in
release notes.
--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 10:13:48AM -0400, Chapman Flack wrote: > On 07/23/2018 10:01 AM, Bruce Momjian wrote: > > > And the larger question is whether a patent free for use by software > > under any license can be used in a defensive way. If not, it means we > > have no way forward here. > > Isn't 'defensive', in patent-speak, used to mean 'establishing prior > art usable to challenge future patent claims by others on the same > technique'? > > Is there any way that conditions of use, or lack of them, on an > existing patent, would make it unusable in that context? It doesn't have to be a patent on the same technique; this URL was referenced in the thread: https://en.wikipedia.org/wiki/Defensive_termination -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: [HACKERS] logical decoding of two-phase transactions
On 2018-07-23 19:37:46 +0530, Nikhil Sontakke wrote:
> Hi Andres,
>
> >> > what I'm proposing is that that various catalog access functions throw a
> >> > new class of error, something like "decoding aborted transactions".
> >>
> >> When will this error be thrown by the catalog functions? How will it
> >> determine that it needs to throw this error?
> >
> > The error check would have to happen at the end of most systable_*
> > functions. They'd simply do something like
> >
> > if (decoding_in_progress_xact && TransactionIdDidAbort(xid_of_aborted))
> >ereport(ERROR, (errcode(DECODING_ABORTED_XACT), errmsg("oops")));
> >
> > i.e. check whether the transaction to be decoded still is in
> > progress. As that would happen before any potentially wrong result can
> > be returned (as the check happens at the tail end of systable_*),
> > there's no issue with wrong state in the syscache etc.
> >
>
> Oh, ok. The systable_* functions use the passed in snapshot and return
> tuples matching to it. They do not typically have access to the
> current XID being worked upon..
That seems like quite a solvable issue, especially compared to the
locking schemes proposed.
> We can find out if the snapshot is a logical decoding one by virtue of
> its "satisfies" function pointing to HeapTupleSatisfiesHistoricMVCC.
I think we even can just do something like a global
TransactionId check_if_transaction_is_alive = InvalidTransactionId;
and just set it up during decoding. And then just check it whenever it's
not set tot InvalidTransactionId.
Greetings,
Andres Freund
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 07:08:32AM -0700, Andres Freund wrote: > On 2018-07-23 09:56:47 -0400, Bruce Momjian wrote: > > They can say whatever they want, but if they are bankrupt, what they say > > doesn't matter much. My guess is that they would have to give their > > patents to some legal entity that owns them so it is shielded from > > bankrupcy. > > Huh? Bancruptcy doesn't simply invalidate licenses which successors then > can ignore. By that logic a license to use the code, like the PG > license, would be just as ineffectual You are not thinking this through. It depends if the patent owner retains some un-released/un-shared rights to the patent, e.g. patent can only be used by others in GPL-licensed code. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: Add SKIP LOCKED to VACUUM and ANALYZE
On 7/22/18, 10:12 PM, "Michael Paquier" wrote:
> The refactoring for CLUSTER is pretty obvious, and makes the API a bit
> cleaner, so attached is a proposal of patch to do so. Thoughts?
Sorry for the delay on these patches! This is nearly identical to
what I started writing last night, so it looks good to me.
+typedef enum ClusterOption
+{
+ CLUOPT_RECHECK, /* recheck relation state */
+ CLUOPT_VERBOSE /* print progress info */
+} ClusterOption;
It looks like the last line here has a bit of extra whitespace
compared to the other code in parsenodes.h.
Nathan
Re: How can we submit code patches that implement our (pending) patents?
On 2018-07-23 10:27:10 -0400, Bruce Momjian wrote: > On Mon, Jul 23, 2018 at 07:08:32AM -0700, Andres Freund wrote: > > On 2018-07-23 09:56:47 -0400, Bruce Momjian wrote: > > > They can say whatever they want, but if they are bankrupt, what they say > > > doesn't matter much. My guess is that they would have to give their > > > patents to some legal entity that owns them so it is shielded from > > > bankrupcy. > > > > Huh? Bancruptcy doesn't simply invalidate licenses which successors then > > can ignore. By that logic a license to use the code, like the PG > > license, would be just as ineffectual > > You are not thinking this through. Err. > It depends if the patent owner retains some un-released/un-shared > rights to the patent, e.g. patent can only be used by others in > GPL-licensed code. Please point me to any sort of reference that bancruptcy simply terminates perpetual irrevocable license agreements; that's simply not the case afaik. Even if the patent rights are liquidated, the new owner would be bound by the terms. Also, as I pointed out above, how would the same not be applicable to the code itself? Greetings, Andres Freund
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 06:31:14AM -0700, Andres Freund wrote: > On July 23, 2018 6:25:42 AM PDT, Bruce Momjian wrote: > >Notice this makes no mention of what happens to the patents if the > >company goes bankrupt. My guess is that in such a situation the > >company > >would have no control over who buys the patents or how they are used. > > It explicitly says irrevocable and successors. Why seems squarely > aimed at your concern. Bankruptcy wouldn't just invalidate that. Until this has been upheld in court, it's just a vague idea. > Similarly icenses like Apache 2 grant a perpetual and irrevocable > patent grant for the use in the contribution. The "upheld in court" business applies here, too. I know it's tempting to look at formal-looking texts and infer that they operate something vaguely like computer code, but that is manifestly not the case. Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: [HACKERS] possible self-deadlock window after bad ProcessStartupPacket
On Fri, Jul 20, 2018 at 4:53 AM, Heikki Linnakangas wrote: > ISTM that no-one has any great ideas on what to do about the ereport() in > quickdie(). But I think we have consensus on replacing the exit(2) calls > with _exit(2). If we do just that, it would be better than the status quo, > even if it doesn't completely fix the problem. This would prevent the case > that Asim reported, for starters. +1 for trying to improve this by using _exit rather than exit, and for not letting the perfect be the enemy of the good. But -1 for copying the language "if some idiot DBA sends a manual SIGQUIT to a random backend". I think that phrase could be deleted from this comment -- and all of the other places where this comment appears already today -- without losing any useful informational content. Or it could be rephrased to "if this process receives a SIGQUIT". It's just not necessary to call somebody an idiot to communicate the point. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 07:32:34AM -0700, Andres Freund wrote: > On 2018-07-23 10:27:10 -0400, Bruce Momjian wrote: > > On Mon, Jul 23, 2018 at 07:08:32AM -0700, Andres Freund wrote: > > > On 2018-07-23 09:56:47 -0400, Bruce Momjian wrote: > > > > They can say whatever they want, but if they are bankrupt, what they say > > > > doesn't matter much. My guess is that they would have to give their > > > > patents to some legal entity that owns them so it is shielded from > > > > bankrupcy. > > > > > > Huh? Bancruptcy doesn't simply invalidate licenses which successors then > > > can ignore. By that logic a license to use the code, like the PG > > > license, would be just as ineffectual > > > > You are not thinking this through. > > Err. > > > > It depends if the patent owner retains some un-released/un-shared > > rights to the patent, e.g. patent can only be used by others in > > GPL-licensed code. > > Please point me to any sort of reference that bancruptcy simply > terminates perpetual irrevocable license agreements; that's simply not > the case afaik. Even if the patent rights are liquidated, the new owner > would be bound by the terms. > Also, as I pointed out above, how would the same not be applicable to > the code itself? I am explaining a case where some of the patent rights are given to a group, but some rights are retained by the company. They cannot change the rights given, but can change how the company-controlled rights are used. I am just guessing since I have not heard of any such cases, but I know that companies have limited rights now their assets are sold. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: Indicate anti-wraparound autovacuum in log_autovacuum_min_duration
On Sat, Jul 21, 2018 at 4:22 AM, Michael Paquier wrote: > On Sat, Jul 21, 2018 at 09:38:38AM +0300, Sergei Kornilov wrote: >> Currently log_autovacuum_min_duration log message has no difference >> between regular autovacuum and to prevent wraparound autovacuum. There >> are important differences, for example, backend can automatically >> cancel regular autovacuum, but not anti-wraparound. I think it is >> useful indicate anti-wraparound in logs. > > Yes, a bit more verbosity would be nice for that. Using the term > "anti-wraparound" directly in the logs makes the most sense? I'm not particularly in love with that terminology. I doubt that it's very clear to the average user. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Log query parameters for terminated execute
Hello 23.07.2018, 17:08, "Tom Lane" : > Sergei Kornilov writes: >> Please test with logging command tag %i in log_line_prefix. Extended >> protocol has three different messages, each can be canceled by timeout. But >> here is completely no parameters in PARSE and i did not change BIND in first >> patch. > > This patch scares me to death. It risks calling user-defined I/O > functions in all sorts of weird places, particularly outside transactions, > or in already-failed transactions, or with no ActiveSnapshot. This is reason why i start thread with question how do it right way As i wrote in beginning: > i have no good idea how print ParamListInfo correctly. We can not use > OidOutputFunctionCall in all cases, right? Attached patch is just simple enough to illustrate one possible way. I can further work with proper design, but i need idea how it should look. > Separately from that concern: it appears to result in a substantial > degradation of existing functionality in the places where you did > s/errdetail/errdetail_log/. What was the reason for that? This is my second question at thread beginning. Why used errdetail? We assume that the user wants to get their own parameters back (if he set client_min_messages to LOG)? regards, Sergei
Re: How can we submit code patches that implement our (pending) patents?
On 07/23/2018 10:25 AM, Bruce Momjian wrote: >> Isn't 'defensive', in patent-speak, used to mean 'establishing prior >> art usable to challenge future patent claims by others on the same >> technique'? >> >> Is there any way that conditions of use, or lack of them, on an >> existing patent, would make it unusable in that context? > > It doesn't have to be a patent on the same technique; this URL was > referenced in the thread: > > https://en.wikipedia.org/wiki/Defensive_termination Ah, a very different understanding of defensive use of a patent, and one that I can see would lose force if there could be no conditions on its use. I was thinking more of the use of a filing to establish prior art so somebody else later can't obtain and enforce a patent on the technique that you're already using. Something along the lines of a statutory invention registration[1], which used to be a thing in the US, but now apparently is not, though any filed, published application, granted or abandoned, can serve the same purpose. That, I think, would still work. -Chap [1] https://en.wikipedia.org/wiki/United_States_Statutory_Invention_Registration
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 10:42:11AM -0400, Chapman Flack wrote: > On 07/23/2018 10:25 AM, Bruce Momjian wrote: > > >> Isn't 'defensive', in patent-speak, used to mean 'establishing prior > >> art usable to challenge future patent claims by others on the same > >> technique'? > >> > >> Is there any way that conditions of use, or lack of them, on an > >> existing patent, would make it unusable in that context? > > > > It doesn't have to be a patent on the same technique; this URL was > > referenced in the thread: > > > > https://en.wikipedia.org/wiki/Defensive_termination > > Ah, a very different understanding of defensive use of a patent, > and one that I can see would lose force if there could be no > conditions on its use. > > I was thinking more of the use of a filing to establish prior art > so somebody else later can't obtain and enforce a patent on > the technique that you're already using. Something along the lines > of a statutory invention registration[1], which used to be a thing > in the US, but now apparently is not, though any filed, published > application, granted or abandoned, can serve the same purpose. > > That, I think, would still work. Yes, those preemptive patent approaches work too. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: How can we submit code patches that implement our (pending) patents?
Hi, On 2018-07-23 16:32:55 +0200, David Fetter wrote: > On Mon, Jul 23, 2018 at 06:31:14AM -0700, Andres Freund wrote: > > On July 23, 2018 6:25:42 AM PDT, Bruce Momjian wrote: > > >Notice this makes no mention of what happens to the patents if the > > >company goes bankrupt. My guess is that in such a situation the > > >company > > >would have no control over who buys the patents or how they are used. > > > > It explicitly says irrevocable and successors. Why seems squarely > > aimed at your concern. Bankruptcy wouldn't just invalidate that. > > Until this has been upheld in court, it's just a vague idea. To my knowledge this has long been settled. Which makes a lot of sense, because this is relevant *far* beyond just open source. FWIW, in the US it's explicit law, see https://www.law.cornell.edu/uscode/text/11/365 subclause (n). Anyway, should we decide that this would be a good idea as a policy matter, we'd *OBVIOUSLY* have to check in with lawyers to see whether our understanding is correct. But that doesn't mean we should just assume it's impossible without any sort of actual understanding. You're just muddying the waters. Greetings, Andres Freund
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 07:59:20AM -0700, Andres Freund wrote: > Hi, > > On 2018-07-23 16:32:55 +0200, David Fetter wrote: > > On Mon, Jul 23, 2018 at 06:31:14AM -0700, Andres Freund wrote: > > > On July 23, 2018 6:25:42 AM PDT, Bruce Momjian wrote: > > > >Notice this makes no mention of what happens to the patents if the > > > >company goes bankrupt. My guess is that in such a situation the > > > >company > > > >would have no control over who buys the patents or how they are used. > > > > > > It explicitly says irrevocable and successors. Why seems squarely > > > aimed at your concern. Bankruptcy wouldn't just invalidate that. > > > > Until this has been upheld in court, it's just a vague idea. > > To my knowledge this has long been settled. Which makes a lot of sense, > because this is relevant *far* beyond just open source. FWIW, in the US > it's explicit law, see https://www.law.cornell.edu/uscode/text/11/365 > subclause (n). Anyway, should we decide that this would be a good idea > as a policy matter, we'd *OBVIOUSLY* have to check in with lawyers to > see whether our understanding is correct. But that doesn't mean we > should just assume it's impossible without any sort of actual > understanding. You are saying that Red Hat's promise is part of the contract when they contributed that code --- I guess that interpretation is possible, but again, I am not sure. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: Alter index rename concurrently to
On 23.07.18 15:14, Andrey Klychkov wrote: > Moreover, if you rename Table without query locking, it may crushes your > services that > do queries at the same time, therefore, this is unlikely that someone > will be do it > with concurrent queries to renamed table, in other words, with running > production. > So, I think it doesn't have real sense to use the lower lock for example > for tables (at least by default). > However, renaming Indexes with the lower lock is safe for database consumers > because they don't know anything about them. You appear to be saying that you think that renaming an index concurrently is not safe. In that case, this patch should be rejected. However, I don't think it necessarily is unsafe. What we need is some reasoning about the impact, not a bunch of different options that we don't understand. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Have an encrypted pgpass file
On Wed, Jul 18, 2018 at 11:19 PM, Tom Lane wrote: > Sorry, I don't buy that line of argument. The *only* reason for this > feature to exist is if it allows ready creation of security solutions > that are actually more secure than a non-world-readable .pgpass file. > That's a much higher bar than many people realize to begin with ... > and if it comes along with huge risk of security foot-guns, I do not > think that it's going to be a net advance. I don't think I agree with this objection. First, not doing anything won't be a net advance, either. Second, your objection seems akin to saying "we're not going to let you drive because you might crash the car". There are *some* people who should not be allowed to get behind the wheel, but your proposal seems analogous to banning *everyone* from driving on the theory that car crashes are bad. I think that's an overreaction. I agree that there's probably a risk, but why can't we just document best practices? Really, I'm not sure that it's right to suppose that you're calling a shell script specifically. If it's a Perl, Python, Ruby, etc. script the risk is probably much less -- you're going to take $ARGV[1] or the equivalent and shove it in a string variable, and after that it's not really any more or less risky than any other string variable you've got. You could of course perform an ill-considered interpolation into a shell command, but that's true of any string that originates from a user in any situation, and if you're a somewhat-knowledgeable programmer you probably won't. Generally you have to do *extra* work to make things safe in the shell, whereas in a scripting language you just have to not screw up. foo($thingy) is safe in Perl; foo $thingy is unsafe in the shell. Of course mistakes are possible and we can avoid all the mistakes by not providing the feature, but to me, that doesn't seem like the way to go. > One reason I'd like to see a concrete use-case (or several concrete > use-cases) is that we might then find some design that's less prone > to such mistakes than "here, run this shell script" is going to be. I think that the most common use case is likely to be to get the data from a local or remote keyserver. For example, when I'm logged in, my keychain is available to provide passwords; when I log out, those passwords aren't accessible any more. Or, when the server is in the datacenter where it's supposed to be located, it can pull the data from some other machine in that data center whose job it is provide said data; when the server is physically stolen from the datacenter and taken to some other location, the other machine isn't there and necessary credentials are no longer available (or even if the other machine *is* there, it probably requires a manually-entered password to start the key service, which the thief may not have). > I'm vaguely imagining exec'ing a program directly without a layer > of shell quoting/evaluation in between; but not sure how far that > gets us. It's not a bad thought, although it might not help that much if it causes somebody who would have written PGPASSCOMMAND="this that" to instead set PGPASSCOMMAND="thisthat.sh" where that file contains #!/bin/bash this that ...which seems like a likely outcome. > Another question that ought to be asked somewhere along here is > "how well does this work on Windows?" ... True. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 07:59:20AM -0700, Andres Freund wrote: > Hi, > > On 2018-07-23 16:32:55 +0200, David Fetter wrote: > > On Mon, Jul 23, 2018 at 06:31:14AM -0700, Andres Freund wrote: > > > On July 23, 2018 6:25:42 AM PDT, Bruce Momjian wrote: > > > >Notice this makes no mention of what happens to the patents if the > > > >company goes bankrupt. My guess is that in such a situation the > > > >company > > > >would have no control over who buys the patents or how they are used. > > > > > > It explicitly says irrevocable and successors. Why seems squarely > > > aimed at your concern. Bankruptcy wouldn't just invalidate that. > > > > Until this has been upheld in court, it's just a vague idea. > > To my knowledge this has long been settled. Which makes a lot of sense, > because this is relevant *far* beyond just open source. FWIW, in the US > it's explicit law, see https://www.law.cornell.edu/uscode/text/11/365 > subclause (n). Anyway, should we decide that this would be a good idea > as a policy matter, we'd *OBVIOUSLY* have to check in with lawyers to > see whether our understanding is correct. But that doesn't mean we > should just assume it's impossible without any sort of actual > understanding. Yet again, you are assuming contrary to reality that you can simply read and understand how legal code will operate without court cases to back it. In the particular instance you're citing, that's what happens *after* the contract is upheld as legal, which it could well not be. There are lots of ways it might not be, even if you don't count bad will and venue shopping, tactics known to be used ubiquitously by NPEs. We know things about the GPL because courts have upheld provisions in it, not because somebody's idea of the "obvious meaning" of that document held sway. > You're just muddying the waters. Nope. I'm just making sure we understand that when it comes to patent thickets, it's a LOT easier to get in than out. Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: How can we submit code patches that implement our (pending) patents?
On 2018-07-23 11:06:25 -0400, Bruce Momjian wrote: > On Mon, Jul 23, 2018 at 07:59:20AM -0700, Andres Freund wrote: > > Hi, > > > > On 2018-07-23 16:32:55 +0200, David Fetter wrote: > > > On Mon, Jul 23, 2018 at 06:31:14AM -0700, Andres Freund wrote: > > > > On July 23, 2018 6:25:42 AM PDT, Bruce Momjian wrote: > > > > >Notice this makes no mention of what happens to the patents if the > > > > >company goes bankrupt. My guess is that in such a situation the > > > > >company > > > > >would have no control over who buys the patents or how they are used. > > > > > > > > It explicitly says irrevocable and successors. Why seems squarely > > > > aimed at your concern. Bankruptcy wouldn't just invalidate that. > > > > > > Until this has been upheld in court, it's just a vague idea. > > > > To my knowledge this has long been settled. Which makes a lot of sense, > > because this is relevant *far* beyond just open source. FWIW, in the US > > it's explicit law, see https://www.law.cornell.edu/uscode/text/11/365 > > subclause (n). Anyway, should we decide that this would be a good idea > > as a policy matter, we'd *OBVIOUSLY* have to check in with lawyers to > > see whether our understanding is correct. But that doesn't mean we > > should just assume it's impossible without any sort of actual > > understanding. > > You are saying that Red Hat's promise is part of the contract when they > contributed that code --- I guess that interpretation is possible, but > again, I am not sure. I'm fairly sure that I'm right. But my point isn't that we should "trust Andres implicitly ™" (although that's obviously not a bad starting point ;)). But rather, given that that is a reasonable assumption that such agreements are legally possible, we can decide whether we want to take advantage of such terms *assuming they are legally sound*. Then, if, and only if, we decide that that's interesting from a policy POV, we can verify those assumptions with lawyers. Given we're far from the first project dealing with this, and that companies that have shown themselves to be reasonably trustworthy around open source, like Red Hat, assuming that such agreements are sound seems quite reasonable. I find it fairly hubristic to just assume bad faith, or lack of skill, on part of the drafters of Apache2, GLPv3, RH patent promise, ... that they either didn't think about bancruptcy or didn't care about it. They're certainly better lawyers than any of us here. Greetings, Andres Freund
Re: Remove psql's -W option
On Sun, Jul 22, 2018 at 9:35 AM, Fabien COELHO wrote: > Otherwise ISTM that "-W/--password" still has some minimal value thus does > not deserve to be thrown out that quickly. I think I agree. I don't think this option is really hurting anything, so I'm not quite sure why we would want to abruptly get rid of it. I also think your other question is a good one. It seems like the fact that we need to reconnect -- rather than just prompting for the password and then sending it when we get it -- is an artifact of how libpq is designed rather than an intrinsic limitation of the protocol. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Should contrib modules install .h files?
On 23.07.18 06:15, Tom Lane wrote: > Michael Paquier writes: >> On Sun, Jul 22, 2018 at 09:42:08PM -0400, Stephen Frost wrote: >>> So, +1 from me for having a directory for each extension. > >> So, like Stephen, that's a +1 from me. > > Same here. One-file-per-extension is too strongly biased to tiny > extensions (like most of our contrib examples). Nobody said anything about one-file-per-extension. You can of course have hstore_this.h and hstore_that.h or if you want to have many, use postgis/this.h and postgis/that.h. That's how every C package in the world works. We don't need to legislate further here other than, use sensible naming. Also, let's recall that the point of this exercise is that you want to install the header files so that you can build things (another extension) that somehow interacts with those extensions. Then, even if you put things in separate directories per extension, you still need to make sure that all the installed header files don't clash, since you'll be adding the -I options of several of them. In a way, doing it this way will make things less robust, since it will appear to give extension authors license to use generic header names. > I don't have a real strong opinion on whether it's too late to > push this into v11. I do not think it'd break anything other than > packagers' lists of files to be installed ... but it does seem > like a new feature, and we're past feature freeze. Certainly a new feature. I suggest submitting it to the next commit fest. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: How can we submit code patches that implement our (pending) patents?
Hi, On 2018-07-23 17:11:30 +0200, David Fetter wrote: > Yet again, you are assuming contrary to reality that you can simply > read and understand how legal code will operate without court cases to > back it. Oh, FFS. You're implying serious bad faith here (and not just on my part, but also on the drafters of apache2, gplv3 etc). I *explicitly* said that we should check in with lawyers if we decided we want to go forward. You're arguing that we shouldn't even consider anything around patent grants because YOU don't know of caselaw supporting what's proposed. You're very clearly not a lawyer. You're doing precisely what you warn about. > In the particular instance you're citing, that's what happens > *after* the contract is upheld as legal, which it could well not be. In which case we'd likely be screwed anyway, because the right to the code would quite likely not be effective anymore either. A lot of the relevant law treats copyright and patents similarly. Greetings, Andres Freund
Re: Remove psql's -W option
On Mon, Jul 23, 2018 at 11:20:46AM -0400, Robert Haas wrote: > On Sun, Jul 22, 2018 at 9:35 AM, Fabien COELHO wrote: > > Otherwise ISTM that "-W/--password" still has some minimal value thus does > > not deserve to be thrown out that quickly. > > I think I agree. I don't think this option is really hurting > anything, so I'm not quite sure why we would want to abruptly get rid > of it. > > I also think your other question is a good one. It seems like the > fact that we need to reconnect -- rather than just prompting for the > password and then sending it when we get it -- is an artifact of how > libpq is designed rather than an intrinsic limitation of the protocol. Am I understanding correctly that doing the following would be acceptable, assuming good code quality? - Rearrange libpq so it doesn't force this behavior. - Deprecate the -W option uniformly in the code we ship by documenting it and making it send warnings to stderr. Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack
On Thu, Jul 19, 2018 at 7:17 PM, Jeremy Schneider wrote: > I'd like to bump this old bug that Lloyd filed for more discussion. It > seems serious enough to me that we should at least talk about it. > > Anyone with simply the login privilege and the ability to run SQL can > instantly block all new incoming connections to a DB including new > superuser connections. > > session 1: > select pg_sleep(99) from pg_stat_activity; > > session 2: > vacuum full pg_authid; -or- truncate table pg_authid; > > (there are likely other SQL you could run in session 2 as well.) ExecuteTruncate needs to be refactored to use RangeVarGetRelidExtended with a non-NULL callback rather than heap_openrv, and expand_vacuum_rel needs to use RangeVarGetRelidExtended with a callback instead of RangeVarGetRelid. See cbe24a6dd8fb224b9585f25b882d5ffdb55a0ba5 as an example of what to do. I fixed a large number of cases of this problem back around that time, but then ran out of steam and had to move onto other things before I got them all. Patches welcome. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Should contrib modules install .h files?
> "Peter" == Peter Eisentraut writes: Peter> Nobody said anything about one-file-per-extension. You can of Peter> course have hstore_this.h and hstore_that.h or if you want to Peter> have many, use postgis/this.h and postgis/that.h. So now you want the extension to be able to _optionally_ specify a subdirectory? (just having the extension do HEADERS=foo/bar.h will not work, because as with every other similar makefile variable, that means "install the file foo/bar.h as bar.h, not as foo/bar.h".) -- Andrew (irc:RhodiumToad)
Re: pgbench-ycsb
On Sun, Jul 22, 2018 at 4:42 PM, Fabien COELHO wrote: > Basically I'm against having something called YCSB if it is not YCSB;-) Yep, that seems pretty clear. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 08:19:35AM -0700, Andres Freund wrote: > I'm fairly sure that I'm right. But my point isn't that we should "trust > Andres implicitly ™" (although that's obviously not a bad starting point > ;)). But rather, given that that is a reasonable assumption that such > agreements are legally possible, we can decide whether we want to take > advantage of such terms *assuming they are legally sound*. Then, if, and > only if, we decide that that's interesting from a policy POV, we can > verify those assumptions with lawyers. > > Given we're far from the first project dealing with this, and that > companies that have shown themselves to be reasonably trustworthy around > open source, like Red Hat, assuming that such agreements are sound seems > quite reasonable. Sun Microsystems seemed reasonably trustworthy too. > I find it fairly hubristic to just assume bad faith, or lack of skill, > on part of the drafters of Apache2, GLPv3, RH patent promise, ... that > they either didn't think about bankruptcy or didn't care about > it. They're certainly better lawyers than any of us here. True. It would be nice if we could get an answer about bankruptcy from Red Hat. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: [HACKERS] logical decoding of two-phase transactions
Hi Andres, >> We can find out if the snapshot is a logical decoding one by virtue of >> its "satisfies" function pointing to HeapTupleSatisfiesHistoricMVCC. > > I think we even can just do something like a global > TransactionId check_if_transaction_is_alive = InvalidTransactionId; > and just set it up during decoding. And then just check it whenever it's > not set tot InvalidTransactionId. > > Ok. I will work on something along these lines and re-submit the set of patches. Regards, Nikhils
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 11:40:41AM -0400, Bruce Momjian wrote: > On Mon, Jul 23, 2018 at 08:19:35AM -0700, Andres Freund wrote: > > I'm fairly sure that I'm right. But my point isn't that we should "trust > > Andres implicitly ™" (although that's obviously not a bad starting point > > ;)). But rather, given that that is a reasonable assumption that such > > agreements are legally possible, we can decide whether we want to take > > advantage of such terms *assuming they are legally sound*. Then, if, and > > only if, we decide that that's interesting from a policy POV, we can > > verify those assumptions with lawyers. > > > > > Given we're far from the first project dealing with this, and that > > companies that have shown themselves to be reasonably trustworthy around > > open source, like Red Hat, assuming that such agreements are sound seems > > quite reasonable. > > Sun Microsystems seemed reasonably trustworthy too. I realize what you are saying is that at the time Red Hat wrote that, they had good intentions, but they might not be able to control its behavior in a bankruptcy, so didn't mention it. Also, Oracle is suing Google over the Java API: https://en.wikipedia.org/wiki/Oracle_America,_Inc._v._Google,_Inc. which I can't imagine Sun doing, but legally Oracle can now that they own Java via Sun. Of course, Sun might not have realized the problem, and Red Hat might have, but that's also assuming that there aren't other problems that Red Hat doesn't know about. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: How can we submit code patches that implement our (pending) patents?
Hi, On 2018-07-23 11:40:41 -0400, Bruce Momjian wrote: > Sun Microsystems seemed reasonably trustworthy too. I don't really agree with that characterization (they've a long history of weird behaviour around open source, LONG before the Oracle acquisition). But it doesn't really matter, as they've not breached any of their hard obligations, have they? Greetings, Andres Freund
Re: cached plans and enable_partition_pruning
On Mon, Jul 23, 2018 at 11:20 PM, Andres Freund wrote: > Hi, > > On 2018-07-23 18:31:43 +0900, Amit Langote wrote: >> It seems that because enable_partition_pruning's value is only checked >> during planning, turning it off *after* a plan is created and cached does >> not work as expected. [ ... ] >> Should we check its value during execution too, as done in the attached? > > I think it's correct to check the plan time value, rather than the > execution time value. Other enable_* GUCs also take effect there, and I > don't see a problem with that? Ah, so that may have been intentional. Although, I wonder if enable_partition_pruning could be made to work differently than other enable_* settings, because we *can* perform pruning which is an optimization function even during execution, whereas we cannot modify the plan in other cases? Thanks, Amit
Re: pgbench - remove double declaration of hash functions
On Sun, Jul 22, 2018 at 7:14 PM, Fabien COELHO wrote: > I noticed that hash functions appear twice in the list of pgbench functions, > although once is enough. The code is functional nevertheless, but it looks > silly. This was added by "e51a04840a1" back in March, so should be removed > from 11 and 12dev. Good catch. Committed and back-patched. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: cached plans and enable_partition_pruning
On 2018-Jul-24, Amit Langote wrote: > On Mon, Jul 23, 2018 at 11:20 PM, Andres Freund wrote: > > I think it's correct to check the plan time value, rather than the > > execution time value. Other enable_* GUCs also take effect there, and I > > don't see a problem with that? > > Ah, so that may have been intentional. Although, I wonder if > enable_partition_pruning could be made to work differently than other > enable_* settings, because we *can* perform pruning which is an > optimization function even during execution, whereas we cannot modify > the plan in other cases? Well, let's discuss the use-case for doing that. We introduced the GUC to cover for the case of bugs in the pruning code (and even then there was people saying we should remove it.) Why would you have the GUC turned on during planning but off during execution? -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Remove psql's -W option
Robert Haas writes: > I also think your other question is a good one. It seems like the > fact that we need to reconnect -- rather than just prompting for the > password and then sending it when we get it -- is an artifact of how > libpq is designed rather than an intrinsic limitation of the protocol. Well, it's a limitation of the libpq API. The problem is that it's the application, not libpq, that's in charge of actually asking the user for a password. Right now we inform the app that it needs to do that by passing back a failed PGconn with appropriate state. We could imagine passing back a PGconn with a half-finished open connection, and asking the app to re-submit that PGconn along with a password so we could continue the auth handshake. But it'd require changing apps to do that. Also, doing things like that would incur the risk of exceeding authentication_timeout while the user is typing his password. So we'd also need some additional complexity to retry in that situation. regards, tom lane
Re: [HACKERS] logical decoding of two-phase transactions
On Thu, Jul 19, 2018 at 3:42 PM, Andres Freund wrote: > I don't think this reasoning actually applies for making HOT pruning > weaker as necessary for decoding. The xmin horizon on catalog tables is > already pegged, which'd prevent similar problems. That sounds completely wrong to me. Setting the xmin horizon keeps tuples that are made dead by a committing transaction from being removed, but I don't think it will do anything to keep tuples that are made dead by an aborting transaction from being removed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Should contrib modules install .h files?
Peter Eisentraut writes: > Also, let's recall that the point of this exercise is that you want to > install the header files so that you can build things (another > extension) that somehow interacts with those extensions. Then, even if > you put things in separate directories per extension, you still need to > make sure that all the installed header files don't clash, since you'll > be adding the -I options of several of them. In a way, doing it this > way will make things less robust, since it will appear to give extension > authors license to use generic header names. Personally, I'd recommend using *one* -I switch and having .c files reference extension headers with #include "extensionname/headername.h". As I said before, I think that we should change the existing contrib modules to be coded likewise, all using a single -I switch that points at SRCDIR/contrib. That'd help give people the right coding model to follow. regards, tom lane
Re: [HACKERS] logical decoding of two-phase transactions
On July 23, 2018 9:11:13 AM PDT, Robert Haas wrote: >On Thu, Jul 19, 2018 at 3:42 PM, Andres Freund >wrote: >> I don't think this reasoning actually applies for making HOT pruning >> weaker as necessary for decoding. The xmin horizon on catalog tables >is >> already pegged, which'd prevent similar problems. > >That sounds completely wrong to me. Setting the xmin horizon keeps >tuples that are made dead by a committing transaction from being >removed, but I don't think it will do anything to keep tuples that are >made dead by an aborting transaction from being removed. My point is that we could just make HTSV treat them as recently dead, without incurring the issues of the bug you referenced. Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 09:56:47AM -0400, Bruce Momjian wrote: > On Mon, Jul 23, 2018 at 06:31:14AM -0700, Andres Freund wrote: > > It explicitly says irrevocable and successors. Why seems squarely > > aimed at your concern. Bankruptcy wouldn't just invalidate that. > > They can say whatever they want, but if they are bankrupt, what they say > doesn't matter much. My guess is that they would have to give their > patents to some legal entity that owns them so it is shielded from > bankrupcy. Can you explain how a new owner could invalidate/revoke previous irrevocable grants? That's not rhetorical. I want to know if that's possible. Perhaps patent law [in some countries] requires contracts as opposed to licenses? Nico --
Re: Should contrib modules install .h files?
> "Tom" == Tom Lane writes: Tom> As I said before, I think that we should change the existing Tom> contrib modules to be coded likewise, all using a single -I switch Tom> that points at SRCDIR/contrib. That'd help give people the right Tom> coding model to follow. I don't see that playing nicely with PGXS? -- Andrew.
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 10:13:48AM -0400, Chapman Flack wrote: > On 07/23/2018 10:01 AM, Bruce Momjian wrote: > > > And the larger question is whether a patent free for use by software > > under any license can be used in a defensive way. If not, it means we > > have no way forward here. > > Isn't 'defensive', in patent-speak, used to mean 'establishing prior > art usable to challenge future patent claims by others on the same > technique'? Not prior art. You don't need patents to establish prior art. Just publish your idea and you're done. (The problem with just publishing an idea is that someone might have a patent application regarding a similar idea and might modify their application to cover yours once they see it. Later, when you go claim that you have prior art, there will be a heavy burden of presumption on you. This, of course, encourages you to apply for a patent...) Patents let you have counter-suit options should you get sued for patent infringement -- that's the "defensive" in defensive patents. You need a large portfolio of patents, and this doesn't work against patent trolls. There's also no-lawsuit covenants in grants. You get to use my patent if you don't sue me for using yours, and if you sue me then my grants to you are revoked. Nico --
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 11:40:41AM -0400, Bruce Momjian wrote: > On Mon, Jul 23, 2018 at 08:19:35AM -0700, Andres Freund wrote: > > I'm fairly sure that I'm right. But my point isn't that we should "trust > > Andres implicitly ™" (although that's obviously not a bad starting point > > ;)). But rather, given that that is a reasonable assumption that such > > agreements are legally possible, we can decide whether we want to take > > advantage of such terms *assuming they are legally sound*. Then, if, and > > only if, we decide that that's interesting from a policy POV, we can > > verify those assumptions with lawyers. > > > > > Given we're far from the first project dealing with this, and that > > companies that have shown themselves to be reasonably trustworthy around > > open source, like Red Hat, assuming that such agreements are sound seems > > quite reasonable. > > Sun Microsystems seemed reasonably trustworthy too. Are there patent grants from Sun that Oracle has attempted to renege on? Are there court cases about that? Links? Nico --
Re: [HACKERS] logical decoding of two-phase transactions
On Mon, Jul 23, 2018 at 12:13 PM, Andres Freund wrote: > My point is that we could just make HTSV treat them as recently dead, without > incurring the issues of the bug you referenced. That doesn't seem sufficient. For example, it won't keep the predecessor tuple's ctid field from being overwritten by a subsequent updater -- and if that happens then the update chain is broken. Maybe your idea of cross-checking at the end of each syscache lookup would be sufficient to prevent that from happening, though. But I wonder if there are subtler problems, too -- e.g. relfrozenxid vs. actual xmins in the table, clog truncation, or whatever. There might be no problem, but the idea that an aborted transaction is of no further interest to anybody is pretty deeply ingrained in the system. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 11:55:01AM -0400, Bruce Momjian wrote: > On Mon, Jul 23, 2018 at 11:40:41AM -0400, Bruce Momjian wrote: > > On Mon, Jul 23, 2018 at 08:19:35AM -0700, Andres Freund wrote: > > > I'm fairly sure that I'm right. But my point isn't that we should "trust > > > Andres implicitly ™" (although that's obviously not a bad starting point > > > ;)). But rather, given that that is a reasonable assumption that such > > > agreements are legally possible, we can decide whether we want to take > > > advantage of such terms *assuming they are legally sound*. Then, if, and > > > only if, we decide that that's interesting from a policy POV, we can > > > verify those assumptions with lawyers. > > > > > > > > Given we're far from the first project dealing with this, and that > > > companies that have shown themselves to be reasonably trustworthy around > > > open source, like Red Hat, assuming that such agreements are sound seems > > > quite reasonable. > > > > Sun Microsystems seemed reasonably trustworthy too. > > I realize what you are saying is that at the time Red Hat wrote that, > they had good intentions, but they might not be able to control its > behavior in a bankruptcy, so didn't mention it. Also, Oracle is suing > Google over the Java API: > > https://en.wikipedia.org/wiki/Oracle_America,_Inc._v._Google,_Inc. > > which I can't imagine Sun doing, but legally Oracle can now that they > own Java via Sun. Of course, Sun might not have realized the problem, > and Red Hat might have, but that's also assuming that there aren't other > problems that Red Hat doesn't know about. That's not about patents though, is it. (I do believe that case is highly contrived. Sun put Java under the GPL, so presumably Google can fork it under those terms. I've not followed that case, so I don't really know what's up with it or why it wasn't just dismissed with prejudice.) Nico --
Re: Stored procedures and out parameters
On Mon, Jul 23, 2018 at 2:23 AM, Shay Rojansky wrote:
> Hi hackers, I've encountered some odd behavior with the new stored procedure
> feature, when using INOUT parameters, running PostgreSQL 11-beta2.
>
> With the following procedure:
>
> CREATE OR REPLACE PROCEDURE my_proc(INOUT results text)
> LANGUAGE 'plpgsql'
> AS $BODY$
> BEGIN
> select 'test' into results;
> END;
> $BODY$;
>
> executing CALL my_proc('whatever') yields a resultset with a "results"
> column and a single row, containing "test". This is expected and is also how
> functions work.
>
> However, connecting via Npgsql, which uses the extended protocol, I see
> something quite different. As a response to a Describe PostgreSQL message, I
> get back a NoData response rather than a RowDescription message, In other
> words, it would seem that the behavior of stored procedures differs between
> the simple and extended protocols, when INOUT parameters are involved.
I might be wrong, but that sounds like a bug.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 11:27:49AM -0500, Nico Williams wrote: > On Mon, Jul 23, 2018 at 09:56:47AM -0400, Bruce Momjian wrote: > > On Mon, Jul 23, 2018 at 06:31:14AM -0700, Andres Freund wrote: > > > It explicitly says irrevocable and successors. Why seems squarely > > > aimed at your concern. Bankruptcy wouldn't just invalidate that. > > > > They can say whatever they want, but if they are bankrupt, what they say > > doesn't matter much. My guess is that they would have to give their > > patents to some legal entity that owns them so it is shielded from > > bankruptcy. > > Can you explain how a new owner could invalidate/revoke previous > irrevocable grants? The question is whether the promises Red Hat makes are part of the license for free use of their patents or something they fully control and just promise to do, i.e. is this a promise from the company or embedded in the patent grant. I don't know. But it is a larger question of how such a promise is embedded in the patent grant and _cannot_ be revoked, even if someone else buys the patent in a bankruptcy case and does control that promise. > That's not rhetorical. I want to know if that's possible. > > Perhaps patent law [in some countries] requires contracts as opposed to > licenses? Yes, I really don't know. I have just seen enough "oh, we didn't think of that" to be cautious. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 11:37:05AM -0500, Nico Williams wrote: > On Mon, Jul 23, 2018 at 11:40:41AM -0400, Bruce Momjian wrote: > > On Mon, Jul 23, 2018 at 08:19:35AM -0700, Andres Freund wrote: > > > I'm fairly sure that I'm right. But my point isn't that we should "trust > > > Andres implicitly ™" (although that's obviously not a bad starting point > > > ;)). But rather, given that that is a reasonable assumption that such > > > agreements are legally possible, we can decide whether we want to take > > > advantage of such terms *assuming they are legally sound*. Then, if, and > > > only if, we decide that that's interesting from a policy POV, we can > > > verify those assumptions with lawyers. > > > > > > > > Given we're far from the first project dealing with this, and that > > > companies that have shown themselves to be reasonably trustworthy around > > > open source, like Red Hat, assuming that such agreements are sound seems > > > quite reasonable. > > > > Sun Microsystems seemed reasonably trustworthy too. > > Are there patent grants from Sun that Oracle has attempted to renege on? > Are there court cases about that? Links? No, but I bet there are things Oracle is doing that no one at Sun expected to be done, and users who relied on Sun didn't expect to be done. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: How can we submit code patches that implement our (pending) patents?
On Mon, Jul 23, 2018 at 11:38:47AM -0500, Nico Williams wrote: > On Mon, Jul 23, 2018 at 11:55:01AM -0400, Bruce Momjian wrote: > > On Mon, Jul 23, 2018 at 11:40:41AM -0400, Bruce Momjian wrote: > > > On Mon, Jul 23, 2018 at 08:19:35AM -0700, Andres Freund wrote: > > > > I'm fairly sure that I'm right. But my point isn't that we should "trust > > > > Andres implicitly ™" (although that's obviously not a bad starting point > > > > ;)). But rather, given that that is a reasonable assumption that such > > > > agreements are legally possible, we can decide whether we want to take > > > > advantage of such terms *assuming they are legally sound*. Then, if, and > > > > only if, we decide that that's interesting from a policy POV, we can > > > > verify those assumptions with lawyers. > > > > > > > > > > > Given we're far from the first project dealing with this, and that > > > > companies that have shown themselves to be reasonably trustworthy around > > > > open source, like Red Hat, assuming that such agreements are sound seems > > > > quite reasonable. > > > > > > Sun Microsystems seemed reasonably trustworthy too. > > > > I realize what you are saying is that at the time Red Hat wrote that, > > they had good intentions, but they might not be able to control its > > behavior in a bankruptcy, so didn't mention it. Also, Oracle is suing > > Google over the Java API: > > > > https://en.wikipedia.org/wiki/Oracle_America,_Inc._v._Google,_Inc. > > > > which I can't imagine Sun doing, but legally Oracle can now that they > > own Java via Sun. Of course, Sun might not have realized the problem, > > and Red Hat might have, but that's also assuming that there aren't other > > problems that Red Hat doesn't know about. > > That's not about patents though, is it. No, it is not. It is just a case of not being able to trust any company's "good will". You only get what the contract says. > (I do believe that case is highly contrived. Sun put Java under the > GPL, so presumably Google can fork it under those terms. I've not > followed that case, so I don't really know what's up with it or why it > wasn't just dismissed with prejudice.) Yes, it is a big case with big ramifications if upheld. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: Stored procedures and out parameters
> "Robert" == Robert Haas writes: >> However, connecting via Npgsql, which uses the extended protocol, I >> see something quite different. As a response to a Describe >> PostgreSQL message, I get back a NoData response rather than a >> RowDescription message, In other words, it would seem that the >> behavior of stored procedures differs between the simple and >> extended protocols, when INOUT parameters are involved. Robert> I might be wrong, but that sounds like a bug. Completely off the cuff, I'd expect 59a85323d might have fixed that; does it fail on the latest 11-stable? -- Andrew (irc:RhodiumToad)
Re: GiST VACUUM
Hi! > 21 июля 2018 г., в 17:11, Andrey Borodin написал(а): > > <0001-Physical-GiST-scan-in-VACUUM-v13.patch> Just in case, here's second part of patch series with actual page deletion. I was considering further decreasing memory footprint by using bloom filters instead of bitmap, but it will create seriously more work for cpu to compute hashes. Best regards, Andrey Borodin. 0002-Delete-pages-during-GiST-VACUUM-v13.patch Description: Binary data 0001-Physical-GiST-scan-in-VACUUM-v13.patch Description: Binary data
