Re: Add A Glossary
On 2020-May-17, Jürgen Purtz wrote: > On 15.05.20 02:00, Alvaro Herrera wrote: > > Thanks everybody. I have compiled together all the suggestions and the > > result is in the attached patch. Some of it is of my own devising. > > > > * I changed "instance", and made "cluster" be mostly a synonym of that. > In my understanding, "instance" and "cluster" should be different things, > not only synonyms. "instance" can be the term for permanently fluctuating > objects (processes and RAM) and "cluster" can denote the more static objects > (directories and files). What do you think? If you agree, I would create a > patch. I don't think that's the general understanding of those terms. For all I know, they *are* synonyms, and there's no specific term for "the fluctuating objects" as you call them. The instance is either running (in which case there are processes and RAM) or it isn't. > > * I removed "global SQL object" and made "SQL object" explain it. > +1., but see the (huge) different spellings in patch. This seems a misunderstanding of what "local" means. Any object that exists in a database is local, regardless of whether it exists in a schema or not. "Extensions" is one type of object that does not belong in a schema. "Foreign data wrapper" is another type of object that does not belong in a schema. Same with data type casts. They are *not* global objects. > bloat: changed 'current row' to 'relevant row' because not only the youngest > one is relevant (non-bloat). Hm. TBH I'm not sure of this term at all. I think we sometimes use the term "bloat" to talk about the dead rows only, ignoring the free space. > data type casts: Are you sure that they are global? In pg_cast 'relisshared' > is 'false'. I'm not saying they're global. I'm saying they're outside schemas. Maybe this definition needs more rewording, if this bit is unclear. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Add A Glossary
On 15.05.20 02:00, Alvaro Herrera wrote: Thanks everybody. I have compiled together all the suggestions and the result is in the attached patch. Some of it is of my own devising. * I changed "instance", and made "cluster" be mostly a synonym of that. In my understanding, "instance" and "cluster" should be different things, not only synonyms. "instance" can be the term for permanently fluctuating objects (processes and RAM) and "cluster" can denote the more static objects (directories and files). What do you think? If you agree, I would create a patch. * I removed "global SQL object" and made "SQL object" explain it. +1., but see the (huge) different spellings in patch. bloat: changed 'current row' to 'relevant row' because not only the youngest one is relevant (non-bloat). data type casts: Are you sure that they are global? In pg_cast 'relisshared' is 'false'. -- Jürgen Purtz diff --git a/doc/src/sgml/glossary.sgml b/doc/src/sgml/glossary.sgml index 8bb1ea5d87..75f0dc9a8c 100644 --- a/doc/src/sgml/glossary.sgml +++ b/doc/src/sgml/glossary.sgml @@ -179,7 +179,7 @@ Bloat - Space in data pages which does not contain current row versions, + Space in data pages which does not contain relevant row versions, such as unused (free) space or outdated row versions. @@ -1388,23 +1388,27 @@ Any object that can be created with a CREATE - command. Most objects are specific to one database, and are commonly - known as local objects. + command. Most objects are specific to one schema within + one database, and are commonly + known as local SQL objects. + + + Some of the SQL objects do not belong to a single schema but + are known in all schemas of a database. Examples are + extensions like + foreign data wrappers, and + data type casts. + + + Some others even belong to the complete cluster and are + known in all databases and all its schemas. Roles, tablespaces, replication origins, subscriptions for logical replication, and - databases themselves are not local SQL objects since they exist - entirely outside of any specific database; - they are called global objects. + all database names exist + entirely outside of any specific database. + They are called global SQL objects. - - Most local objects belong to a specific - schema in their containing database. - There also exist local objects that do not belong to schemas; some examples are - extensions, - data type casts, and - foreign data wrappers. - For more information, see .
Re: [HACKERS] Restricting maximum keep segments by repslots
Hi, On 2020-05-16 22:51:50 -0400, Alvaro Herrera wrote: > On 2020-May-16, Andres Freund wrote: > > > I, independent of this patch, added a few additional paths in which > > checkpointer's latch is reset, and I found a few shutdowns in regression > > tests to be extremely slow / timing out. The reason for that is that > > the only check for interrupts is at the top of the loop. So if > > checkpointer gets SIGUSR2 we don't see ShutdownRequestPending until we > > decide to do a checkpoint for other reasons. > > Ah, yeah, this seems a genuine bug. > > > I also suspect that it could have harmful consequences to not do a > > AbsorbSyncRequests() if something "ate" the set latch. > > I traced through this when looking over the previous fix, and given that > checkpoint execution itself calls AbsorbSyncRequests frequently, I > don't think this one qualifies as a bug. There's no AbsorbSyncRequests() after CheckPointBuffers(), I think. And e.g. CheckPointTwoPhase() could take a while. Which then would mean that we'd potentially not AbsorbSyncRequests() until checkpoint_timeout causes us to wake up. Am I missing something? > > One way to do that would be to WaitLatch() call to much earlier, and > > only do a WaitLatch() if do_checkpoint is false. Roughly like in the > > attached. > > Hm. I'd do "WaitLatch() / continue" in the "!do_checkpoint" block, and > put the checpkoint code not in the else block; seems easier to read to > me. Yea, that'd probably be better. I was also pondering if we shouldn't just move the checkpoint code into, gasp, it's own function ;) Greetings, Andres Freund
Re: [HACKERS] Restricting maximum keep segments by repslots
On 2020-May-16, Andres Freund wrote: > I, independent of this patch, added a few additional paths in which > checkpointer's latch is reset, and I found a few shutdowns in regression > tests to be extremely slow / timing out. The reason for that is that > the only check for interrupts is at the top of the loop. So if > checkpointer gets SIGUSR2 we don't see ShutdownRequestPending until we > decide to do a checkpoint for other reasons. Ah, yeah, this seems a genuine bug. > I also suspect that it could have harmful consequences to not do a > AbsorbSyncRequests() if something "ate" the set latch. I traced through this when looking over the previous fix, and given that checkpoint execution itself calls AbsorbSyncRequests frequently, I don't think this one qualifies as a bug. > I don't think it's reasonable to expect this much code between a > ResetLatch and WaitLatch to never reset a latch. So I think we need to > make the coding more robust in face of that. Without having to duplicate > the top and the bottom of the loop. That makes sense to me. > One way to do that would be to WaitLatch() call to much earlier, and > only do a WaitLatch() if do_checkpoint is false. Roughly like in the > attached. Hm. I'd do "WaitLatch() / continue" in the "!do_checkpoint" block, and put the checpkoint code not in the else block; seems easier to read to me. While we're here, can we change CreateCheckPoint to return true so that we can do ckpt_performed = do_restartpoint ? CreateRestartPoint(flags) : CreateCheckPoint(flags); instead of the mess we have there now? (Also add a comment that CreateCheckPoint must not return false, to avoid messing with the schedule) -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Add A Glossary
On 2020-May-16, Erik Rijkers wrote: > On 2020-05-15 19:26, Alvaro Herrera wrote: > > Applied all these suggestions, and made a few additional very small > > edits, and pushed -- better to ship what we have now in beta1, but > > further edits are still possible. > > I've gone through the glossary as committed and found some more small > things; patch attached. All pushed! Many thanks, -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Potentially misleading name of libpq pass phrase hook
On 5/16/20 7:47 PM, Tom Lane wrote: > Daniel Gustafsson writes: >>> On 16 May 2020, at 03:56, Michael Paquier wrote: >>> Agreed. PQsslKeyPassHook__type sounds fine to me as >>> convention. Wouldn't we want to also rename PQsetSSLKeyPassHook and >>> PQgetSSLKeyPassHook, appending an "_OpenSSL" to both? >> Yes, I think we should. The attached performs the rename of the hook >> functions >> and the type, and also fixes an off-by-one-'=' in a header comment which my >> OCD >> couldn't unsee. > The patch as committed missed renaming PQgetSSLKeyPassHook() itself, > but did rename its result type, which seemed to me to be clearly > wrong. I took it on myself to fix that up, and also to fix exports.txt > which some of the buildfarm insists be correct ;-) > > argh! thanks! cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Performance penalty when requesting text values in binary format
I'm the creator of the PostgreSQL driver pgx (https://github.com/jackc/pgx) for the Go language. I have found significant performance advantages to using the extended protocol and binary format values -- in particular for types such as timestamptz. However, I was recently very surprised to find that it is significantly slower to select a text type value in the binary format. For an example case of selecting 1,000 rows each with 5 text columns of 16 bytes each the application time from sending the query to having received the entire response is approximately 16% slower. Here is a link to the test benchmark: https://github.com/jackc/pg_text_binary_bench Given that the text and binary formats for the text type are identical I would not have expected any performance differences. My C is rusty and my knowledge of the PG server internals is minimal but the performance difference appears to be that function textsend creates an extra copy where textout simply returns a pointer to the existing data. This seems to be superfluous. I can work around this by specifying the format per result column instead of specifying binary for all but this performance bug / anomaly seemed worth reporting. Jack
Re: pg_stat_wal_receiver and flushedUpto/writtenUpto
On Sat, May 16, 2020 at 10:15:47AM +0900, Michael Paquier wrote: > Thanks. If there are no objections, I'll revisit that tomorrow and > apply it with those changes, just in time for beta1. Okay, done this part then. -- Michael signature.asc Description: PGP signature
Re: [HACKERS] Restricting maximum keep segments by repslots
Hi,
On 2020-04-29 18:58:16 -0400, Alvaro Herrera wrote:
> On 2020-Apr-28, Alvaro Herrera wrote:
>
> > On 2020-Apr-28, Kyotaro Horiguchi wrote:
> >
> > > > Anyway I think this patch should fix it also -- instead of adding a new
> > > > flag, we just rely on the existing flags (since do_checkpoint must have
> > > > been set correctly from the flags earlier in that block.)
> > >
> > > Since the added (!do_checkpoint) check is reached with
> > > do_checkpoint=false at server start and at archive_timeout intervals,
> > > the patch makes checkpointer run a busy-loop at that timings, and that
> > > loop lasts until a checkpoint is actually executed.
> > >
> > > What we need to do here is not forgetting the fact that the latch has
> > > been set even if the latch itself gets reset before reaching to
> > > WaitLatch.
> >
> > After a few more false starts :-) I think one easy thing we can do
> > without the additional boolean flag is to call SetLatch there in the
> > main loop if we see that ckpt_flags is nonzero.
>
> I went back to "continue" instead of SetLatch, because it seems less
> wasteful, but I changed the previously "do_checkpoint" condition to
> rechecking ckpt_flags. We would not get in the busy loop in that case,
> because the condition is true when the next loop would take action and
> false otherwise. So I think this should fix the problem without causing
> any other issues. But if you do see problems with this, please let us
> know.
I don't think this is quite sufficient:
I, independent of this patch, added a few additional paths in which
checkpointer's latch is reset, and I found a few shutdowns in regression
tests to be extremely slow / timing out. The reason for that is that
the only check for interrupts is at the top of the loop. So if
checkpointer gets SIGUSR2 we don't see ShutdownRequestPending until we
decide to do a checkpoint for other reasons.
I also suspect that it could have harmful consequences to not do a
AbsorbSyncRequests() if something "ate" the set latch.
I don't think it's reasonable to expect this much code between a
ResetLatch and WaitLatch to never reset a latch. So I think we need to
make the coding more robust in face of that. Without having to duplicate
the top and the bottom of the loop.
One way to do that would be to WaitLatch() call to much earlier, and
only do a WaitLatch() if do_checkpoint is false. Roughly like in the
attached.
Greetings,
Andres Freund
diff --git i/src/backend/postmaster/checkpointer.c w/src/backend/postmaster/checkpointer.c
index 2b552a7ff90..92e420eb052 100644
--- i/src/backend/postmaster/checkpointer.c
+++ w/src/backend/postmaster/checkpointer.c
@@ -353,6 +353,18 @@ CheckpointerMain(void)
BgWriterStats.m_requested_checkpoints++;
}
+ /* Check for archive_timeout and switch xlog files if necessary. */
+ CheckArchiveTimeout();
+
+ /*
+ * Send off activity statistics to the stats collector. (The reason
+ * why we re-use bgwriter-related code for this is that the bgwriter
+ * and checkpointer used to be just one process. It's probably not
+ * worth the trouble to split the stats support into two independent
+ * stats message types.)
+ */
+ pgstat_send_bgwriter();
+
/*
* Force a checkpoint if too much time has elapsed since the last one.
* Note that we count a timed checkpoint in stats only when this
@@ -370,9 +382,29 @@ CheckpointerMain(void)
}
/*
- * Do a checkpoint if requested.
+ * Do a checkpoint if requested, or wait until it's time to do so.
*/
- if (do_checkpoint)
+ if (!do_checkpoint)
+ {
+ /*
+ * Sleep until we are signaled or it's time for another checkpoint or
+ * xlog file switch.
+ */
+ cur_timeout = CheckPointTimeout - elapsed_secs;
+ if (XLogArchiveTimeout > 0 && !RecoveryInProgress())
+ {
+elapsed_secs = now - last_xlog_switch_time;
+if (elapsed_secs >= XLogArchiveTimeout)
+ continue; /* no sleep for us ... */
+cur_timeout = Min(cur_timeout, XLogArchiveTimeout - elapsed_secs);
+ }
+
+ (void) WaitLatch(MyLatch,
+ WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ cur_timeout * 1000L /* convert to ms */ ,
+ WAIT_EVENT_CHECKPOINTER_MAIN);
+ }
+ else
{
bool ckpt_performed = false;
bool do_restartpoint;
@@ -481,47 +513,6 @@ CheckpointerMain(void)
ckpt_active = false;
}
-
- /* Check for archive_timeout and switch xlog files if necessary. */
- CheckArchiveTimeout();
-
- /*
- * Send off activity statistics to the stats collector. (The reason
- * why we re-use bgwriter-related code for this is that the bgwriter
- * and checkpointer used to be just one process. It's probably not
- * worth the trouble to split the stats support into two independent
- * stats message types.)
- */
- pgstat_send_bgwriter();
-
- /*
- * If any checkpoint flags have been set, redo the loop to handle the
- * checkpoint without sleeping.
- */
- if (((volatile CheckpointerShmemStruct *) Checkpo
Re: pg_bsd_indent and -Wimplicit-fallthrough
On Sat, May 16, 2020 at 11:56:28AM -0400, Tom Lane wrote: > In the meantime, I went ahead and pushed this to our pg_bsd_indent repo. Thanks, Tom. -- Michael signature.asc Description: PGP signature
Re: Potentially misleading name of libpq pass phrase hook
Daniel Gustafsson writes: >> On 16 May 2020, at 03:56, Michael Paquier wrote: >> Agreed. PQsslKeyPassHook__type sounds fine to me as >> convention. Wouldn't we want to also rename PQsetSSLKeyPassHook and >> PQgetSSLKeyPassHook, appending an "_OpenSSL" to both? > Yes, I think we should. The attached performs the rename of the hook > functions > and the type, and also fixes an off-by-one-'=' in a header comment which my > OCD > couldn't unsee. The patch as committed missed renaming PQgetSSLKeyPassHook() itself, but did rename its result type, which seemed to me to be clearly wrong. I took it on myself to fix that up, and also to fix exports.txt which some of the buildfarm insists be correct ;-) regards, tom lane
Re: [PATCH] fix GIN index search sometimes losing results
I wrote:
> I think the root of the problem is that if we have a query using
> weights, and we are testing tsvector data that lacks positions/weights,
> we can never say there's definitely a match. I don't see any decently
> clean way to fix this without redefining the TSExecuteCallback API
> to return a tri-state YES/NO/MAYBE result, because really we need to
> decide that it's MAYBE at the level of processing the QI_VAL node,
> not later on. I'd tried to avoid that in e81e5741a, but maybe we
> should just bite that bullet, and not worry about whether there's
> any third-party code providing its own TSExecuteCallback routine.
> codesearch.debian.net suggests that there are no external callers
> of TS_execute, so maybe we can get away with that.
0001 attached is a proposed patch that does it that way. Given the
API break involved, it's not quite clear what to do with this.
ISTM we have three options:
1. Ignore the API issue and back-patch. Given the apparent lack of
external callers of TS_execute, maybe we can get away with that;
but I wonder if we'd get pushback from distros that have automatic
ABI-break detectors in place.
2. Assume we can't backpatch, but it's still OK to slip this into
v13. (This option clearly has a limited shelf life, but I think
we could get away with it until late beta.)
3. Assume we'd better hold this till v14.
I find #3 unduly conservative, seeing that this is clearly a bug
fix, but on the other hand #1 is a bit scary. Aside from the API
issue, it's not impossible that this has introduced some corner
case behavioral changes that we'd consider to be new bugs rather
than bug fixes.
Anyway, some notes for reviewers:
* The core idea of the patch is to make the TS_execute callbacks
have ternary results and to insist they return TS_MAYBE in any
case where the correct result is uncertain.
* That fixes the bug at hand, and it also allows getting rid of
some kluges at higher levels. The GIN code no longer needs its
own TS_execute_ternary implementation, and the GIST code no longer
needs to suppose that it can't trust NOT results.
* I put some effort into not leaking memory within tsvector_op.c's
checkclass_str and checkcondition_str. (The final output array
can still get leaked, I believe. Fixing that seems like material
for a different patch, and it might not be worth any trouble.)
* The new test cases in tstypes.sql are to verify that we didn't
change behavior of the basic tsvector @@ tsquery code. There wasn't
any coverage of these cases before, and the logic for checkclass_str
without position info had to be tweaked to preserve this behavior.
* The new cases in tsearch verify that the GIN and GIST code gives
the same results as the basic operator.
Now, as for the 0002 patch attached: after 0001, the only TS_execute()
callers that are not specifying TS_EXEC_CALC_NOT are hlCover(),
which I'd already complained is probably a bug, and the first of
the two calls in tsrank.c's Cover(). It seems difficult to me to
argue that it's not a bug for Cover() to process NOT in one call
but not the other --- moreover, if there was any argument for that
once upon a time, it probably falls to the ground now that (a) we
have a less buggy implementation of NOT and (b) the presence of
phrase queries significantly raises the importance of not taking
short-cuts. Therefore, 0002 attached rips out the TS_EXEC_CALC_NOT
flag and has TS_execute compute NOT expressions accurately all the
time.
As it stands, 0002 changes no regression test results, which I'm
afraid speaks more to our crummy test coverage than anything else;
tests that exercise those two functions with NOT-using queries
would easily show that there is a difference.
Even if we decide to back-patch 0001, I would not suggest
back-patching 0002, as it's more nearly a definitional change
than a bug fix. But I think it's a good idea anyway.
I'll stick this in the queue for the July commitfest, in case
anybody wants to review it.
regards, tom lane
diff --git a/src/backend/tsearch/wparser_def.c b/src/backend/tsearch/wparser_def.c
index 48e55e1..bfbc8d5 100644
--- a/src/backend/tsearch/wparser_def.c
+++ b/src/backend/tsearch/wparser_def.c
@@ -1969,7 +1969,7 @@ typedef struct
/*
* TS_execute callback for matching a tsquery operand to headline words
*/
-static bool
+static TSTernaryValue
checkcondition_HL(void *opaque, QueryOperand *val, ExecPhraseData *data)
{
hlCheck*checkval = (hlCheck *) opaque;
@@ -1982,7 +1982,7 @@ checkcondition_HL(void *opaque, QueryOperand *val, ExecPhraseData *data)
{
/* if data == NULL, don't need to report positions */
if (!data)
-return true;
+return TS_YES;
if (!data->pos)
{
@@ -1999,9 +1999,9 @@ checkcondition_HL(void *opaque, QueryOperand *val, ExecPhraseData *data)
}
if (data && data->npos > 0)
- return true;
+ return TS_YES;
- return false;
+ return TS_NO;
}
/*
diff --git a/src/backend/utils/adt/tsginidx.c b/src/backe
Re: Add A Glossary
On 2020-05-15 19:26, Alvaro Herrera wrote: Applied all these suggestions, and made a few additional very small edits, and pushed -- better to ship what we have now in beta1, but further edits are still possible. I've gone through the glossary as committed and found some more small things; patch attached. Thanks, Erik Rijkers Other possible terms to define, including those from the tweet I linked to and a couple more: archive availability backup composite type common table expression data type domain dump export fault tolerance GUC high availability hot standby LSN restore secondary server (?) snapshot transactions per second Anybody want to try their hand at a tentative definition? -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services --- doc/src/sgml/glossary.sgml.orig 2020-05-16 23:30:01.132290237 +0200 +++ doc/src/sgml/glossary.sgml 2020-05-16 23:38:35.643371310 +0200 @@ -67,7 +67,7 @@ In reference to a datum: - the fact that its value that cannot be broken down into smaller + the fact that its value cannot be broken down into smaller components. @@ -360,7 +360,7 @@ integrity constraints. Transactions may be allowed to violate some of the constraints transiently before it commits, but if such violations are not resolved - by the time it commits, such transaction is automatically + by the time it commits, such a transaction is automatically rolled back. This is one of the ACID properties. @@ -649,8 +649,8 @@ Grant - An SQL command that is used to allow - users or + An SQL command that is used to allow a + user or role to access specific objects within the database. @@ -887,7 +887,7 @@ A relation that is - defined in the same way that a a view + defined in the same way that a view is, but stores data in the same way that a table does. It cannot be modified via INSERT, UPDATE, or @@ -962,7 +962,7 @@ In reference to a window function: - a partition is a user-defined criteria that identifies which neighboring + a partition is a user-defined criterion that identifies which neighboring rows can be considered by the function. @@ -1446,8 +1446,8 @@ The system catalog resides in the schema pg_catalog. These tables contain data in internal representation and are not typically considered useful for user examination; - a number of user-friendlier views - also in schema pg_catalog offer more convenient access to + a number of user-friendlier views, + also in schema pg_catalog, offer more convenient access to some of that information, while additional tables and views exist in schema information_schema (see ) that expose some @@ -1739,7 +1739,7 @@ each page stores two bits: the first one (all-visible) indicates that all tuples in the page are visible to all transactions. The second one - (all-frozen) indicate that all tuples + (all-frozen) indicates that all tuples in the page are marked frozen. @@ -1755,7 +1755,7 @@ A process that saves copies of WAL files - for the purposes of creating backups or keeping + for the purpose of creating backups or keeping replicas current. @@ -1777,7 +1777,7 @@ and are written in sequential order, interspersing changes as they occur in multiple simultaneous sessions. If the system crashes, the files are read in order, and each of the - changes is replayed to restore the system to the state as it was + changes is replayed to restore the system to the state it was in before the crash.
Warn when parallel restoring a custom dump without data offsets
If pg_dump can't seek on its output stream when writing a dump in the custom archive format (possibly because you piped its stdout to a file) it can't update that file with data offsets. These files will often break parallel restoration. Warn when the user is doing pg_restore on such a file to give them a hint as to why their restore is about to fail. The documentation for pg_restore -j is also updated to suggest that you dump custom archive formats with the -f option. --- doc/src/sgml/ref/pg_restore.sgml | 9 + src/bin/pg_dump/pg_backup_custom.c | 8 2 files changed, 17 insertions(+) 0001-Warn-when-parallel-restoring-a-custom-dump-without-d.patch Description: Binary data
Re: [PATCH] Fix pg_dump --no-tablespaces for the custom format
On Sat, 16 May 2020 at 16:31, Tom Lane wrote: > > I think (b) would be a break in the archive format, with unclear > consequences, so I'm not in favor of that. > > I came to the same conclusion while inspecting the source code. -- Euler Taveira http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Multiple FPI_FOR_HINT for the same block during killing btree index items
Em sex., 15 de mai. de 2020 às 18:53, Alvaro Herrera < alvhe...@2ndquadrant.com> escreveu: > On 2020-Apr-10, Masahiko Sawada wrote: > > > Okay. I think only adding the check would also help with reducing the > > likelihood. How about the changes for the current HEAD I've attached? > > Pushed this to all branches. (Branches 12 and older obviously needed an > adjustment.) Thanks! > > > Related to this behavior on btree indexes, this can happen even on > > heaps during searching heap tuples. To reduce the likelihood of that > > more generally I wonder if we can acquire a lock on buffer descriptor > > right before XLogSaveBufferForHint() and set a flag to the buffer > > descriptor that indicates that we're about to log FPI for hint bit so > > that concurrent process can be aware of that. > > I'm not sure how that helps; the other process would have to go back and > redo their whole operation from scratch in order to find out whether > there's still something alive that needs killing. > > I think you need to acquire the exclusive lock sooner: if, when scanning > the page, you find a killable item, *then* upgrade the lock to exclusive > and restart the scan. This means that we'll have to wait for any other > process that's doing the scan, and they will all give up their share > lock to wait for the exclusive lock they need. So the one that gets it > first will do all the killing, log the page, then release the lock. At > that point the other processes will wake up and see that items have been > killed, so they will return having done nothing. > Regarding the block, I disagree in part, because in the worst case, the block can be requested in the last item analyzed, leading to redo all the work from the beginning. If we are in _bt_killitems it is because there is a high probability that there will be items to be deleted, why not request the block soon, if this meets the conditions? 1. XLogHintBitIsNeeded () 2.! AutoVacuumingActive () 3. New exclusive configuration variable option to activate the lock? Masahiko reported that it occurs only when (autovacuum_enabled = off); regards, Ranier Vilela avoid_killing_btree_items_already_dead_v2.patch Description: Binary data
Re: [PATCH] Fix pg_dump --no-tablespaces for the custom format
Euler Taveira writes: > I'm wondering why there is such a distinction. We have some options: > (a) leave it as is and document that those 2 options has no effect in > pg_dump > and possibly add a warning to report if someone uses it with an archive > format; > (b) exclude owner and tablespace from archive (it breaks compatibility but > do > exactly what users expect). I think throwing a warning saying "this option does nothing" might be a reasonable change. I think (b) would be a break in the archive format, with unclear consequences, so I'm not in favor of that. regards, tom lane
Re: [PATCH] Fix pg_dump --no-tablespaces for the custom format
On Sat, 16 May 2020 at 04:20, Christopher Baines wrote: > > Tom Lane writes: > > > Christopher Baines writes: > >> So I'm new to poking around in the PostgreSQL code, so this is a bit of > >> a shot in the dark. I'm having some problems with pg_dump, and a > >> database with tablespaces. A couple of the tables are not in the default > >> tablespace, and I want to ignore this for the dump. > > > > I think you've misunderstood how the pieces fit together. A lot of > > the detail-filtering switches, including --no-tablespaces, work on > > the output side of the "archive" format. While you can't really tell > > the difference in pg_dump text mode, the implication for custom-format > > output is that the info is always there in the archive file, and you > > give the switch to pg_restore if you don't want to see the info. > > This is more flexible since you aren't compelled to make the decision > > up-front, and it doesn't really cost anything to include such info in > > the archive. (Obviously, table-filtering switches don't work that > > way, since with those there can be a really large cost in file size > > to include unwanted data.) > > > I've also had to explain a dozen times how the archive format works. Archive format is kind of intermediary format because you can produce a plain format using it. [Testing some pg_dump --no-option ...] The following objects are not included if a --no-option is used: * grant / revoke * comment * publication * subscription * security labels but some are included even if --no-option is used: * owner * tablespace I'm wondering why there is such a distinction. We have some options: (a) leave it as is and document that those 2 options has no effect in pg_dump and possibly add a warning to report if someone uses it with an archive format; (b) exclude owner and tablespace from archive (it breaks compatibility but do exactly what users expect). I do not even consider a possibility to include all objects even if a --no-option is used because you will have a bunch of complaints / reports. -- Euler Taveira http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Multiple FPI_FOR_HINT for the same block during killing btree index items
Em sex., 15 de mai. de 2020 às 18:53, Alvaro Herrera < alvhe...@2ndquadrant.com> escreveu: > On 2020-Apr-10, Masahiko Sawada wrote: > > > Okay. I think only adding the check would also help with reducing the > > likelihood. How about the changes for the current HEAD I've attached? > > Pushed this to all branches. (Branches 12 and older obviously needed an > adjustment.) Thanks! > > > Related to this behavior on btree indexes, this can happen even on > > heaps during searching heap tuples. To reduce the likelihood of that > > more generally I wonder if we can acquire a lock on buffer descriptor > > right before XLogSaveBufferForHint() and set a flag to the buffer > > descriptor that indicates that we're about to log FPI for hint bit so > > that concurrent process can be aware of that. > > I'm not sure how that helps; the other process would have to go back and > redo their whole operation from scratch in order to find out whether > there's still something alive that needs killing. > > I think you need to acquire the exclusive lock sooner: if, when scanning > the page, you find a killable item, *then* upgrade the lock to exclusive > and restart the scan. This means that we'll have to wait for any other > process that's doing the scan, and they will all give up their share > lock to wait for the exclusive lock they need. So the one that gets it > first will do all the killing, log the page, then release the lock. At > that point the other processes will wake up and see that items have been > killed, so they will return having done nothing. > > Like the attached. I didn't verify that it works well or that it > actually improves performance ... > This is not related to your latest patch. But I believe I can improve the performance. So: 1. If killedsomething is false 2. Any killtuple is true and (not ItemIdIsDead(iid)) is false 3. Nothing to be done. So why do all the work and then discard it. We can eliminate the current item much earlier, testing if it is already dead. regards, Ranier VIlela avoid_killing_btree_items_aready_dead.patch Description: Binary data
Re: pgindent && weirdness
Thomas Munro writes: > On Sat, May 16, 2020 at 10:15 AM Tom Lane wrote: >> +1. I think the repo will let you in, but if not, I can do it. > It seems I cannot. Please go ahead. Pushed, and I bumped pg_bsd_indent's version to 2.1.1, and synced our core repo with that. regards, tom lane
Re: pg_bsd_indent and -Wimplicit-fallthrough
Julien Rouhaud writes: > On Fri, May 15, 2020 at 9:17 AM Daniel Gustafsson wrote: >> On 15 May 2020, at 08:28, Julien Rouhaud wrote: >>> The patch looks good to me. It looks like we already have custom >>> patches, so +1 to applying it. >> Shouldn't we try and propose it to upstream first to minimize our diff? > Good point, adding Piotr. In the meantime, I went ahead and pushed this to our pg_bsd_indent repo. regards, tom lane
Re: POC: GROUP BY optimization
On Sat, May 16, 2020 at 02:24:31PM +0200, Dmitry Dolgov wrote:
On Fri, May 15, 2020 at 01:52:20AM +0200, Tomas Vondra wrote:
I wonder if anyone has plans to try again with this optimization in v14
cycle? The patches no longer apply thanks to the incremental sort patch,
but I suppose fixing that should not be extremely hard.
The 2020-07 CF is still a couple weeks away, but it'd be good to know if
there are any plans to revive this. I'm willing to spend some time on
reviewing / testing this, etc.
Yes, if you believe that this patch has potential, I would love to pick
it up again.
I think it's worth another try. I've been reminded of this limitation
when working on the incremental sort, which significantly increases the
number of orderings that we can sort efficiently. But we can't possibly
leverage that unless it matches the GROUP BY ordering.
The attached WIP patch somewhat addresses this by trying to reorder the
group_pathkeys so allow leveraging sort of existing ordering (even just
partial, with incremental sort).
I've only quickly skimmed the old thread, but IIRC there were two main
challenges in getting the optimization right:
1) deciding which orderings are interesting / worth additional work
I think we need to consider these orderings, in addition to the one
specified in GROUP BY:
1) as specified in ORDER BY (if different from 1)
What is the idea behind considering this ordering?
I'll try to answer this in general, i.e. what I think this patch needs
to consider. Hopefully that'll answer why we should look at ORDER BY
pathkeys too ...
Reordering the pathkeys has both local and global effects, and we need
to consider all of that to make the optimization reliable, otherwise
we'll inevitably end up with trivial regressions.
The local effects are trivial - it's for example reordering the pathkeys
to make the explicit sort as cheap as possible. This thread already
discussed a number of things to base this on - ndistinct for columns,
cost of comparison function, ... In any case, this is something we can
decide locally, when building the grouping paths.
The global effects are much harder to tackle, because the decision can't
be made locally when building the grouping paths. It requires changes
both below and above the point where we build grouping paths.
An example of a decision we need to make before we even get to building
a grouping path is which index paths to build. Currently we only build
index paths with "useful" pathkeys, and without tweaking that we'll
never even see the index in add_paths_to_grouping_rel().
But there are also decisions that can be made only after we build the
grouping paths. For example, we may have both GROUP BY and ORDER BY, and
there is no "always correct" way to combine those. In some cases it may
be correct to use the same pathkeys, in other cases it's better to use
different ones (which will require an extra Sort, with additional cost).
So I don't think there will be a single "interesting" grouping pathkeys
(i.e. root->group_pathkeys), but a collection of pathkeys. And we'll
need to build grouping paths for all of those, and leave the planner to
eventually pick the one giving us the cheapest plan ...
A "brute-force" approach would be to generate all possible orderings of
group_pathkeys, but that's probably not going to fly - it might easily
cause an explosion in number of paths we track, etc. So we'll need to
pick/prioritize orderings that are more likely to give us efficient
plans, and ORDER BY seems like a good example because it means we won't
need an explicit sort.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/optimizer/path/costsize.c
b/src/backend/optimizer/path/costsize.c
index f4d4a4df66..7a5a3bb73b 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -142,6 +142,7 @@ boolenable_partitionwise_aggregate = false;
bool enable_parallel_append = true;
bool enable_parallel_hash = true;
bool enable_partition_pruning = true;
+bool enable_groupby_reorder = true;
typedef struct
{
diff --git a/src/backend/optimizer/path/pathkeys.c
b/src/backend/optimizer/path/pathkeys.c
index ce9bf87e9b..9d0961dee2 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -22,6 +22,7 @@
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "nodes/plannodes.h"
+#include "optimizer/cost.h"
#include "optimizer/optimizer.h"
#include "optimizer/pathnode.h"
#include "optimizer/paths.h"
@@ -388,6 +389,91 @@ pathkeys_count_contained_in(List *keys1, List *keys2, int
*n_common)
return (key1 == NULL);
}
+/*
+ * pathkeys_maybe_reorder
+ *Reorder pathkeys in keys1 so that the order matches keys2 as much as
+ *possible, i.e. to maximize the common prefix length.
+ */
+List *
+p
Re: Our naming of wait events is a disaster.
I wrote: > Thomas Munro writes: >> On Wed, May 13, 2020 at 3:16 AM Tom Lane wrote: >>> Hash/Batch/Allocating >>> Hash/Batch/Electing >>> Hash/Batch/Loading >>> Hash/GrowBatches/Allocating >> Perhaps we should also drop the 'ing' from the verbs, to be more like >> ...Read etc. > Yeah, that aspect was bothering me too. Comparing these to other > wait event names, you could make a case for either "Allocate" or > "Allocation"; but there are no other names with -ing. After contemplating these for a bit, my proposal is to drop the slashes and convert "verbing" to "verb", giving HashBatchAllocate HashBatchElect HashBatchLoad HashBuildAllocate HashBuildElect HashBuildHashInner HashBuildHashOuter HashGrowBatchesAllocate HashGrowBatchesDecide HashGrowBatchesElect HashGrowBatchesFinish HashGrowBatchesRepartition HashGrowBucketsAllocate HashGrowBucketsElect HashGrowBucketsReinsert In addition to that, I think the ClogGroupUpdate event needs to be renamed to XactGroupUpdate, since we changed "clog" to "xact" in the exposed SLRU and LWLock names. (There are some other names that I wouldn't have picked in a green field, but it's probably not worth the churn to change them.) Also, as previously noted, ProcSignalBarrier should be in the IPC event class not IO. Barring objections, I'll make these things happen before beta1. regards, tom lane
Re: Potentially misleading name of libpq pass phrase hook
On 5/16/20 3:16 AM, Daniel Gustafsson wrote: >> On 16 May 2020, at 03:56, Michael Paquier wrote: >> >> On Fri, May 15, 2020 at 09:21:52PM -0400, Jonathan S. Katz wrote: >>> +1 on all of the above. >>> >>> I noticed this has been added to Open Items; I added a note that the >>> plan is to fix before the Beta 1 wrap. >> >> +1. Thanks. >> >> Agreed. PQsslKeyPassHook__type sounds fine to me as >> convention. Wouldn't we want to also rename PQsetSSLKeyPassHook and >> PQgetSSLKeyPassHook, appending an "_OpenSSL" to both? > > Yes, I think we should. The attached performs the rename of the hook > functions > and the type, and also fixes an off-by-one-'=' in a header comment which my > OCD > couldn't unsee. Reviewed, overall looks good to me. My question is around the name. It appears the convention is to do "openssl" on hooks[1], with the convention being a single hook I could find. But scanning the codebase, it appears we either use "OPENSSL" for definers and "openssl" in function names. So, my 2¢ is to use all lowercase to stick with convention. Thanks! Jonathan [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/include/libpq/libpq-be.h;hb=HEAD#l293 signature.asc Description: OpenPGP digital signature
Re: calling procedures is slow and consumes extra much memory against calling function
Em sáb., 16 de mai. de 2020 às 11:07, Pavel Stehule
escreveu:
>
>
> so 16. 5. 2020 v 15:24 odesílatel Ranier Vilela
> napsal:
>
>> Em sáb., 16 de mai. de 2020 às 09:35, Pavel Stehule <
>> pavel.steh...@gmail.com> escreveu:
>>
>>>
>>>
>>> so 16. 5. 2020 v 13:40 odesílatel Ranier Vilela
>>> napsal:
>>>
Em sáb., 16 de mai. de 2020 às 01:10, Pavel Stehule <
pavel.steh...@gmail.com> escreveu:
>
>
> so 16. 5. 2020 v 5:55 odesílatel Ranier Vilela
> napsal:
>
>> Em sáb., 16 de mai. de 2020 às 00:07, Pavel Stehule <
>> pavel.steh...@gmail.com> escreveu:
>>
>>>
>>>
>>> so 16. 5. 2020 v 0:34 odesílatel Ranier Vilela
>>> napsal:
>>>
Em dom., 10 de mai. de 2020 às 17:21, Pavel Stehule <
pavel.steh...@gmail.com> escreveu:
> Hi
>
> I try to use procedures in Orafce package, and I did some easy
> performance tests. I found some hard problems:
>
> 1. test case
>
> create or replace procedure p1(inout r int, inout v int) as $$
> begin v := random() * r; end
> $$ language plpgsql;
>
> This command requires
>
> do $$
> declare r int default 100; x int;
> begin
> for i in 1..30 loop
> call p1(r, x);
> end loop;
> end;
> $$;
>
> about 2.2GB RAM and 10 sec.
>
I am having a consistent result of 3 secs, with a modified version
(exec_stmt_call) of your patch.
But my notebook is (Core 5, 8GB and SSD), could it be a difference
in the testing hardware?
>>>
>>> My notebook is old T520, and more I have a configured Postgres with
>>> --enable-cassert option.
>>>
>> The hardware is definitely making a difference, but if you have time
>> and don't mind testing it,
>> I can send you a patch, not that the modifications are a big deal,
>> but maybe they'll help.
>>
> With more testing, I found that latency increases response time.
With 3 (secs) the test is with localhost.
With 6 (secs) the test is with tcp (local, not between pcs).
Anyway, I would like to know if we have the number of parameters
previously, why use List instead of Arrays?
It would not be faster to create plpgsql variables.
>>>
>>> Why you check SPI_processed?
>>>
>>> + if (SPI_processed == 1)
>>> + {
>>> + if (!stmt->target)
>>> + elog(ERROR, "DO statement returned a row, query \"%s\"", expr->query);
>>> + }
>>> + else if (SPI_processed > 1)
>>> + elog(ERROR, "Procedure call returned more than one row, query \"%s\"",
>>> expr->query);
>>>
>>>
>>> CALL cannot to return rows, so these checks has not sense
>>>
>> Looking at the original file, this already done, from line 2351,
>> I just put all the tests together to, if applicable, get out quickly.
>>
>
> It's little bit messy. Is not good to mix bugfix and refactoring things
> together
>
Ok, I can understand that.
regards,
Ranier Vilela
Re: calling procedures is slow and consumes extra much memory against calling function
so 16. 5. 2020 v 15:24 odesílatel Ranier Vilela
napsal:
> Em sáb., 16 de mai. de 2020 às 09:35, Pavel Stehule <
> pavel.steh...@gmail.com> escreveu:
>
>>
>>
>> so 16. 5. 2020 v 13:40 odesílatel Ranier Vilela
>> napsal:
>>
>>> Em sáb., 16 de mai. de 2020 às 01:10, Pavel Stehule <
>>> pavel.steh...@gmail.com> escreveu:
>>>
so 16. 5. 2020 v 5:55 odesílatel Ranier Vilela
napsal:
> Em sáb., 16 de mai. de 2020 às 00:07, Pavel Stehule <
> pavel.steh...@gmail.com> escreveu:
>
>>
>>
>> so 16. 5. 2020 v 0:34 odesílatel Ranier Vilela
>> napsal:
>>
>>> Em dom., 10 de mai. de 2020 às 17:21, Pavel Stehule <
>>> pavel.steh...@gmail.com> escreveu:
>>>
Hi
I try to use procedures in Orafce package, and I did some easy
performance tests. I found some hard problems:
1. test case
create or replace procedure p1(inout r int, inout v int) as $$
begin v := random() * r; end
$$ language plpgsql;
This command requires
do $$
declare r int default 100; x int;
begin
for i in 1..30 loop
call p1(r, x);
end loop;
end;
$$;
about 2.2GB RAM and 10 sec.
>>> I am having a consistent result of 3 secs, with a modified version
>>> (exec_stmt_call) of your patch.
>>> But my notebook is (Core 5, 8GB and SSD), could it be a difference
>>> in the testing hardware?
>>>
>>
>> My notebook is old T520, and more I have a configured Postgres with
>> --enable-cassert option.
>>
> The hardware is definitely making a difference, but if you have time
> and don't mind testing it,
> I can send you a patch, not that the modifications are a big deal, but
> maybe they'll help.
>
With more testing, I found that latency increases response time.
>>> With 3 (secs) the test is with localhost.
>>> With 6 (secs) the test is with tcp (local, not between pcs).
>>>
>>> Anyway, I would like to know if we have the number of parameters
>>> previously, why use List instead of Arrays?
>>> It would not be faster to create plpgsql variables.
>>>
>>
>> Why you check SPI_processed?
>>
>> + if (SPI_processed == 1)
>> + {
>> + if (!stmt->target)
>> + elog(ERROR, "DO statement returned a row, query \"%s\"", expr->query);
>> + }
>> + else if (SPI_processed > 1)
>> + elog(ERROR, "Procedure call returned more than one row, query \"%s\"",
>> expr->query);
>>
>>
>> CALL cannot to return rows, so these checks has not sense
>>
> Looking at the original file, this already done, from line 2351,
> I just put all the tests together to, if applicable, get out quickly.
>
It's little bit messy. Is not good to mix bugfix and refactoring things
together
> regards,
> Ranier Vilela
>
valgrind vs force_parallel_mode=regress
As I was resuscitating my buildfarm animal lousyjack, which runs tests under valgrind, I neglected to turn off force_parallel=regress in the configuration settings. The results were fairly disastrous. The runs took about 4 times as long, and some steps failed. I'm not sure if this is a known result, so I thought I'd memorialize it in case someone else runs into the same issue. When I disabled this setting the animal turned to being happy once more. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Potentially misleading name of libpq pass phrase hook
On 5/15/20 8:08 PM, Alvaro Herrera wrote: > On 2020-May-15, Magnus Hagander wrote: > >> On Thu, May 14, 2020 at 3:03 PM Daniel Gustafsson wrote: >>> Since we haven't shipped this there is still time to rename, which >>> IMO is the right way forward. PQsslKeyPassHook__type would >>> be one option, but perhaps there are better alternatives? >> ISTM this should be renamed yeah -- and it should probably go on the open >> item lists, and with the schedule for the beta perhaps dealt with rather >> urgently? > Seems easy enough to do! +1 on Daniel's suggested renaming. > > CCing Andrew as committer. > I'll attend to this today. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: calling procedures is slow and consumes extra much memory against calling function
Em sáb., 16 de mai. de 2020 às 09:35, Pavel Stehule
escreveu:
>
>
> so 16. 5. 2020 v 13:40 odesílatel Ranier Vilela
> napsal:
>
>> Em sáb., 16 de mai. de 2020 às 01:10, Pavel Stehule <
>> pavel.steh...@gmail.com> escreveu:
>>
>>>
>>>
>>> so 16. 5. 2020 v 5:55 odesílatel Ranier Vilela
>>> napsal:
>>>
Em sáb., 16 de mai. de 2020 às 00:07, Pavel Stehule <
pavel.steh...@gmail.com> escreveu:
>
>
> so 16. 5. 2020 v 0:34 odesílatel Ranier Vilela
> napsal:
>
>> Em dom., 10 de mai. de 2020 às 17:21, Pavel Stehule <
>> pavel.steh...@gmail.com> escreveu:
>>
>>> Hi
>>>
>>> I try to use procedures in Orafce package, and I did some easy
>>> performance tests. I found some hard problems:
>>>
>>> 1. test case
>>>
>>> create or replace procedure p1(inout r int, inout v int) as $$
>>> begin v := random() * r; end
>>> $$ language plpgsql;
>>>
>>> This command requires
>>>
>>> do $$
>>> declare r int default 100; x int;
>>> begin
>>> for i in 1..30 loop
>>> call p1(r, x);
>>> end loop;
>>> end;
>>> $$;
>>>
>>> about 2.2GB RAM and 10 sec.
>>>
>> I am having a consistent result of 3 secs, with a modified version
>> (exec_stmt_call) of your patch.
>> But my notebook is (Core 5, 8GB and SSD), could it be a difference in
>> the testing hardware?
>>
>
> My notebook is old T520, and more I have a configured Postgres with
> --enable-cassert option.
>
The hardware is definitely making a difference, but if you have time
and don't mind testing it,
I can send you a patch, not that the modifications are a big deal, but
maybe they'll help.
>>> With more testing, I found that latency increases response time.
>> With 3 (secs) the test is with localhost.
>> With 6 (secs) the test is with tcp (local, not between pcs).
>>
>> Anyway, I would like to know if we have the number of parameters
>> previously, why use List instead of Arrays?
>> It would not be faster to create plpgsql variables.
>>
>
> Why you check SPI_processed?
>
> + if (SPI_processed == 1)
> + {
> + if (!stmt->target)
> + elog(ERROR, "DO statement returned a row, query \"%s\"", expr->query);
> + }
> + else if (SPI_processed > 1)
> + elog(ERROR, "Procedure call returned more than one row, query \"%s\"",
> expr->query);
>
>
> CALL cannot to return rows, so these checks has not sense
>
Looking at the original file, this already done, from line 2351,
I just put all the tests together to, if applicable, get out quickly.
regards,
Ranier Vilela
Re: ldap tls test fails in some environments
On Sat, May 16, 2020 at 2:25 AM Christoph Berg wrote:
> > Somebody should get out the LDAP RFCs and decode the packet contents
> > that this log helpfully provides, but I suspect that we're just looking
> > at an authentication failure; there's still not much clue as to why.
>
> The non-TLS tests work, so it's not a plain auth failure...
>
> I'm attaching the full logs from that test, maybe someone with more
> insight can compare the non-TLS with the TLS bits.
>
> Would it help to re-run that with log_debug on the PG side?
In your transcript for test 20, it looks like the client (PostgreSQL)
is hanging up without even sending a TLS ClientHello:
tls_read: want=5, got=0
...
psql:TLS: can't accept: The TLS connection was non-properly terminated..
With 001_auth.tl hacked to enable debug as you suggested*, on a local
Debian 10 system I see:
tls_read: want=5, got=5
: 16 03 01 01 4c L
That's: 0x16 = handshake record, 0x03 0x01 = TLS 1.0, and then a
record length for the following ClientHello. You're not even getting
that far, so I guess libdap is setting up the connection but then the
GNU TLS library (what Debian links libldap against) is not happy, even
before any negotiations begin. I wonder how to figure out why... does
this tell you anything?
$ENV{'LDAPCONF'} = $ldap_conf;
+$ENV{"GNUTLS_DEBUG_LEVEL"} = '99';
* Like this:
-system_or_bail $slapd, '-f', $slapd_conf, '-h', "$ldap_url $ldaps_url";
+system("$slapd -d 255 -f $slapd_conf -h '$ldap_url $ldaps_url' &");
Re: calling procedures is slow and consumes extra much memory against calling function
so 16. 5. 2020 v 13:40 odesílatel Ranier Vilela
napsal:
> Em sáb., 16 de mai. de 2020 às 01:10, Pavel Stehule <
> pavel.steh...@gmail.com> escreveu:
>
>>
>>
>> so 16. 5. 2020 v 5:55 odesílatel Ranier Vilela
>> napsal:
>>
>>> Em sáb., 16 de mai. de 2020 às 00:07, Pavel Stehule <
>>> pavel.steh...@gmail.com> escreveu:
>>>
so 16. 5. 2020 v 0:34 odesílatel Ranier Vilela
napsal:
> Em dom., 10 de mai. de 2020 às 17:21, Pavel Stehule <
> pavel.steh...@gmail.com> escreveu:
>
>> Hi
>>
>> I try to use procedures in Orafce package, and I did some easy
>> performance tests. I found some hard problems:
>>
>> 1. test case
>>
>> create or replace procedure p1(inout r int, inout v int) as $$
>> begin v := random() * r; end
>> $$ language plpgsql;
>>
>> This command requires
>>
>> do $$
>> declare r int default 100; x int;
>> begin
>> for i in 1..30 loop
>> call p1(r, x);
>> end loop;
>> end;
>> $$;
>>
>> about 2.2GB RAM and 10 sec.
>>
> I am having a consistent result of 3 secs, with a modified version
> (exec_stmt_call) of your patch.
> But my notebook is (Core 5, 8GB and SSD), could it be a difference in
> the testing hardware?
>
My notebook is old T520, and more I have a configured Postgres with
--enable-cassert option.
>>> The hardware is definitely making a difference, but if you have time and
>>> don't mind testing it,
>>> I can send you a patch, not that the modifications are a big deal, but
>>> maybe they'll help.
>>>
>> With more testing, I found that latency increases response time.
> With 3 (secs) the test is with localhost.
> With 6 (secs) the test is with tcp (local, not between pcs).
>
> Anyway, I would like to know if we have the number of parameters
> previously, why use List instead of Arrays?
> It would not be faster to create plpgsql variables.
>
Why you check SPI_processed?
+ if (SPI_processed == 1)
+ {
+ if (!stmt->target)
+ elog(ERROR, "DO statement returned a row, query \"%s\"", expr->query);
+ }
+ else if (SPI_processed > 1)
+ elog(ERROR, "Procedure call returned more than one row, query \"%s\"",
expr->query);
CALL cannot to return rows, so these checks has not sense
> regards,
> Ranier Vilela
>
Re: POC: GROUP BY optimization
> On Fri, May 15, 2020 at 01:52:20AM +0200, Tomas Vondra wrote: > > I wonder if anyone has plans to try again with this optimization in v14 > cycle? The patches no longer apply thanks to the incremental sort patch, > but I suppose fixing that should not be extremely hard. > > The 2020-07 CF is still a couple weeks away, but it'd be good to know if > there are any plans to revive this. I'm willing to spend some time on > reviewing / testing this, etc. Yes, if you believe that this patch has potential, I would love to pick it up again. > I've only quickly skimmed the old thread, but IIRC there were two main > challenges in getting the optimization right: > > > 1) deciding which orderings are interesting / worth additional work > > I think we need to consider these orderings, in addition to the one > specified in GROUP BY: > > 1) as specified in ORDER BY (if different from 1) What is the idea behind considering this ordering?
Re: calling procedures is slow and consumes extra much memory against calling function
Em sáb., 16 de mai. de 2020 às 01:10, Pavel Stehule escreveu: > > > so 16. 5. 2020 v 5:55 odesílatel Ranier Vilela > napsal: > >> Em sáb., 16 de mai. de 2020 às 00:07, Pavel Stehule < >> pavel.steh...@gmail.com> escreveu: >> >>> >>> >>> so 16. 5. 2020 v 0:34 odesílatel Ranier Vilela >>> napsal: >>> Em dom., 10 de mai. de 2020 às 17:21, Pavel Stehule < pavel.steh...@gmail.com> escreveu: > Hi > > I try to use procedures in Orafce package, and I did some easy > performance tests. I found some hard problems: > > 1. test case > > create or replace procedure p1(inout r int, inout v int) as $$ > begin v := random() * r; end > $$ language plpgsql; > > This command requires > > do $$ > declare r int default 100; x int; > begin > for i in 1..30 loop > call p1(r, x); > end loop; > end; > $$; > > about 2.2GB RAM and 10 sec. > I am having a consistent result of 3 secs, with a modified version (exec_stmt_call) of your patch. But my notebook is (Core 5, 8GB and SSD), could it be a difference in the testing hardware? >>> >>> My notebook is old T520, and more I have a configured Postgres with >>> --enable-cassert option. >>> >> The hardware is definitely making a difference, but if you have time and >> don't mind testing it, >> I can send you a patch, not that the modifications are a big deal, but >> maybe they'll help. >> > With more testing, I found that latency increases response time. With 3 (secs) the test is with localhost. With 6 (secs) the test is with tcp (local, not between pcs). Anyway, I would like to know if we have the number of parameters previously, why use List instead of Arrays? It would not be faster to create plpgsql variables. regards, Ranier Vilela pl_exec_test.patch Description: Binary data
Re: Problem with logical replication
On Fri, May 15, 2020 at 08:48:53AM -0300, Euler Taveira wrote: > On Fri, 15 May 2020 at 02:47, Michael Paquier wrote: >> Agreed. I don't think either that we need to update this comment. I >> was playing with this patch and what you have here looks fine by me. >> Two nits: the extra parenthesis in the assert are not necessary, and >> the indentation had some diffs. Tom has just reindented the whole >> tree, so let's keep things clean. > > LGTM. Thanks for double-checking. Applied and back-patched down to 10 then. -- Michael signature.asc Description: PGP signature
Re: Is it useful to record whether plans are generic or custom?
> To track executed plan types, I think execution layer hooks > are appropriate. > These hooks, however, take QueryDesc as a param and it does > not include cached plan information. It seems that the same QueryDesc entry is reused when executing a generic plan. For exemple marking queryDesc->plannedstmt->queryId (outside pg_stat_statements) with a pseudo tag during ExecutorStart reappears in later executions with generic plans ... Is this QueryDesc reusable by a custom plan ? If not maybe a solution could be to add a flag in queryDesc->plannedstmt ? (sorry, I haven't understood yet how and when this generic plan is managed during planning) Regards PAscal -- Sent from: https://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
Re: [PATCH] Fix pg_dump --no-tablespaces for the custom format
Tom Lane writes: > Christopher Baines writes: >> So I'm new to poking around in the PostgreSQL code, so this is a bit of >> a shot in the dark. I'm having some problems with pg_dump, and a >> database with tablespaces. A couple of the tables are not in the default >> tablespace, and I want to ignore this for the dump. > > I think you've misunderstood how the pieces fit together. A lot of > the detail-filtering switches, including --no-tablespaces, work on > the output side of the "archive" format. While you can't really tell > the difference in pg_dump text mode, the implication for custom-format > output is that the info is always there in the archive file, and you > give the switch to pg_restore if you don't want to see the info. > This is more flexible since you aren't compelled to make the decision > up-front, and it doesn't really cost anything to include such info in > the archive. (Obviously, table-filtering switches don't work that > way, since with those there can be a really large cost in file size > to include unwanted data.) > > So from my perspective, things are working fine and this patch would > break it. > > If you actually want to suppress this info from getting into the > archive file, you'd have to give a very compelling reason for > breaking this behavior for other people. Thanks for getting back to me Tom :) I don't really follow how having it do something more along the lines of how it's documented would both be less flexible and break existing uses of pg_dump. You're not prevented from including tablespace assignments, just don't pass --no-tablespaces, and your now able to not include them for the archive formats, just like the plain format, which in my view increases the flexibility of the tool, since something new is possible. I realise that for people who are passing --no-tablespaces, without realising it does nothing combined with the archive formats, that actually not including tablespace assignments will change the behaviour for them, but as above, I'd see this as a positive change, as it makes the tooling more powerful (and simpler to understand as well). I see now that while the --help output doesn't capture the nuances: --no-tablespaces do not dump tablespace assignments The documentation does: --no-tablespaces Do not output commands to select tablespaces. With this option, all objects will be created in whichever tablespace is the default during restore. This option is only meaningful for the plain-text format. For the archive formats, you can specify the option when you call pg_restore. Thanks again, Chris signature.asc Description: PGP signature
Re: Potentially misleading name of libpq pass phrase hook
> On 16 May 2020, at 03:56, Michael Paquier wrote: > > On Fri, May 15, 2020 at 09:21:52PM -0400, Jonathan S. Katz wrote: >> +1 on all of the above. >> >> I noticed this has been added to Open Items; I added a note that the >> plan is to fix before the Beta 1 wrap. > > +1. Thanks. > > Agreed. PQsslKeyPassHook__type sounds fine to me as > convention. Wouldn't we want to also rename PQsetSSLKeyPassHook and > PQgetSSLKeyPassHook, appending an "_OpenSSL" to both? Yes, I think we should. The attached performs the rename of the hook functions and the type, and also fixes an off-by-one-'=' in a header comment which my OCD couldn't unsee. cheers ./daniel openssl_hook_rename.patch Description: Binary data
