Quick doc typo fix
Issue found while translating the v12 manual. I also fixed something that was missing, as far as I understand it (first fix, the typo is the second fix). See patch attached. Thanks. -- Guillaume. diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 4c7e93892a..f81f3d4160 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -57,7 +57,7 @@ pg_am - index access methods + table and index access methods diff --git a/doc/src/sgml/tablesample-method.sgml b/doc/src/sgml/tablesample-method.sgml index 880e42c950..68eea68077 100644 --- a/doc/src/sgml/tablesample-method.sgml +++ b/doc/src/sgml/tablesample-method.sgml @@ -264,7 +264,7 @@ NextSampleTuple (SampleScanState *node, requests to sample missing or invisible tuples; that should not result in any bias in the sample. However, if necessary, the function can use node->donetuples to examine how many of the tuples -it returned were vlaid and visible. +it returned were valid and visible.
Re: Quick doc typo fix
Le mar. 28 mai 2019 à 21:28, Michael Paquier a écrit : > On Tue, May 28, 2019 at 05:05:10PM +0200, Guillaume Lelarge wrote: > > > > linkend="catalog-pg-am">pg_am > > - index access methods > > + table and index access methods > > > > Perhaps we could just say "relation" here? That's the term used on > the paragraph describing pg_am. > Hehe, that was the first thing I wrote :) but went with "table and index" as it was also used a bit later in the chapter. Both are fine with me. -- Guillaume.
Re: Quick doc typo fix
Le mar. 28 mai 2019 à 21:46, Guillaume Lelarge a écrit : > Le mar. 28 mai 2019 à 21:28, Michael Paquier a > écrit : > >> On Tue, May 28, 2019 at 05:05:10PM +0200, Guillaume Lelarge wrote: >> > >> >> linkend="catalog-pg-am">pg_am >> > - index access methods >> > + table and index access methods >> > >> >> Perhaps we could just say "relation" here? That's the term used on >> the paragraph describing pg_am. >> > > Hehe, that was the first thing I wrote :) but went with "table and index" > as it was also used a bit later in the chapter. Both are fine with me. > > And here is another one. See patch attached. -- Guillaume. diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml index 2413089ffe..4c34ad9cc9 100644 --- a/doc/src/sgml/file-fdw.sgml +++ b/doc/src/sgml/file-fdw.sgml @@ -174,7 +174,7 @@ - COPY's FORCE_QUOTE options is + COPY's FORCE_QUOTE option is currently not supported by file_fdw.
Re: Quick doc typo fix
Le mer. 29 mai 2019 19:45, Michael Paquier a écrit : > On Wed, May 29, 2019 at 05:30:33PM +0200, Guillaume Lelarge wrote: > > And here is another one. See patch attached. > > Are you still going through some parts of the documentation? Perhaps > you may notice something else? I am wondering if it would be better > to wait a bit more so as we can group all issues you are finding at > once. > Yeah, I still have quite a lot to process. That might be better to do it all in once.
Re: Quick doc typo fix
Le mer. 29 mai 2019 19:52, Michael Paquier a écrit : > On Wed, May 29, 2019 at 07:47:12PM +0200, Guillaume Lelarge wrote: > > Yeah, I still have quite a lot to process. That might be better to do it > > all in once. > > OK, thanks! Could you ping me on this thread once you think you are > done? > Sure.
Re: Quick doc typo fix
Le sam. 8 juin 2019 à 15:44, Julien Rouhaud a écrit : > On Sat, Jun 8, 2019 at 3:33 PM Michael Paquier > wrote: > > > > Hi Guillaume, > > > > On Wed, May 29, 2019 at 08:28:59PM +0200, Guillaume Lelarge wrote: > > > Sure. > > > > I have noticed your message on the French list about the completion of > > the traduction, and congrats for that, it is a huge amount of work. > > Did you find anything else after your last report? > > I have two more fixes. See attached patch. It was the merge of upstream documentation completion in the french > repo, so the actual translation work can now begin. Probably there > will be more typo finding in the next weeks. > Yeah, only the merge is done. We now need to work on the actual translation. But this is way more fun than the merge :) We might find more typos, but it will take time. Applying this patch now (if it fits you) is probably better. -- Guillaume. diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml index 2413089ffe..4c34ad9cc9 100644 --- a/doc/src/sgml/file-fdw.sgml +++ b/doc/src/sgml/file-fdw.sgml @@ -174,7 +174,7 @@ - COPY's FORCE_QUOTE options is + COPY's FORCE_QUOTE option is currently not supported by file_fdw. diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index e053e2ee34..323621b95c 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -753,7 +753,7 @@ psql: could not connect to server: No such file or directory SEMMNI Maximum number of semaphore identifiers (i.e., sets) -at least ceil((max_connections + autovacuum_max_workers + wax_wal_senders + max_worker_processes + 5) / 16) plus room for other applications +at least ceil((max_connections + autovacuum_max_workers + max_wal_senders + max_worker_processes + 5) / 16) plus room for other applications
Re: Quick doc typo fix
Le dim. 9 juin 2019 à 04:27, Michael Paquier a écrit : > On Sat, Jun 08, 2019 at 04:23:55PM +0200, Guillaume Lelarge wrote: > > We might find more typos, but it will take time. Applying this patch now > > (if it fits you) is probably better. > > I can imagine that it is a daunting task... Ok, for now I have > applied what you sent. Thanks! > Thank you. -- Guillaume.
Re: Quitting the thes
Le mer. 12 juin 2019 à 20:45, Alvaro Herrera a écrit : > On 2019-May-14, Michael Paquier wrote: > > > Thanks, committed. I have noticed an extra one in reorderbuffer.c. > > Some grepping found a bit more; patch attached. > > Thanks a lot, this is very good. I've got some fixes to do :) -- Guillaume.
Re: Extensions not dumped when --schema is used
Le mar. 26 janv. 2021 à 13:42, Guillaume Lelarge a
écrit :
> Le mar. 26 janv. 2021 à 13:41, Guillaume Lelarge
> a écrit :
>
>> Le mar. 26 janv. 2021 à 05:10, Julien Rouhaud a
>> écrit :
>>
>>> On Mon, Jan 25, 2021 at 9:34 PM Guillaume Lelarge
>>> wrote:
>>> >
>>> > "Anytime soon" was a long long time ago, and I eventually completely
>>> forgot this, sorry. As nobody worked on it yet, I took a shot at it. See
>>> attached patch.
>>>
>>> Great!
>>>
>>> I didn't reviewed it thoroughly yet, but after a quick look it sounds
>>> sensible. I'd prefer to see some tests added, and it looks like a
>>> test for plpgsql could be added quite easily.
>>>
>>>
>> I tried that all afternoon yesterday, but failed to do so. My had still
>> hurts, but I'll try again though it may take some time.
>>
>>
> s/My had/My head/ ..
>
>
I finally managed to get a working TAP test for my patch. I have no idea if
it's good, and if it's enough. Anyway, new version of the patch attached.
--
Guillaume.
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index bcbb7a25fb..95d45fabfb 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -215,6 +215,38 @@ PostgreSQL documentation
+
+ -e pattern
+ --extension=pattern
+
+
+Dump only extensions matching pattern. When this option is not
+specified, all non-system extensions in the target database will be
+dumped. Multiple schemas can be selected by writing multiple
+-e switches. The pattern parameter is interpreted as a
+pattern according to the same rules used by
+psql's \d commands (see
+), so multiple extensions can also
+be selected by writing wildcard characters in the pattern. When using
+wildcards, be careful to quote the pattern if needed to prevent the
+shell from expanding the wildcards.
+
+
+
+
+ When -e is specified,
+ pg_dump makes no attempt to dump any other
+ database objects that the selected extension(s) might depend upon.
+ Therefore, there is no guarantee that the results of a
+ specific-extension dump can be successfully restored by themselves
+ into a clean database.
+
+
+
+
+
-E encoding
--encoding=encoding
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index eb988d7eb4..59c4e756ef 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -123,6 +123,9 @@ static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
static SimpleStringList foreign_servers_include_patterns = {NULL, NULL};
static SimpleOidList foreign_servers_include_oids = {NULL, NULL};
+static SimpleStringList extension_include_patterns = {NULL, NULL};
+static SimpleOidList extension_include_oids = {NULL, NULL};
+
static const CatalogId nilCatalogId = {0, 0};
/* override for standard extra_float_digits setting */
@@ -151,6 +154,10 @@ static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
bool strict_names);
+static void expand_extension_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids,
+ bool strict_names);
static void expand_foreign_server_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids);
@@ -334,6 +341,7 @@ main(int argc, char **argv)
{"clean", no_argument, NULL, 'c'},
{"create", no_argument, NULL, 'C'},
{"dbname", required_argument, NULL, 'd'},
+ {"extension", required_argument, NULL, 'e'},
{"file", required_argument, NULL, 'f'},
{"format", required_argument, NULL, 'F'},
{"host", required_argument, NULL, 'h'},
@@ -424,7 +432,7 @@ main(int argc, char **argv)
InitDumpOptions(&dopt);
- while ((c = getopt_long(argc, argv, "abBcCd:E:f:F:h:j:n:N:Op:RsS:t:T:U:vwWxZ:",
+ while ((c = getopt_long(argc, argv, "abBcCd:e:E:f:F:h:j:n:N:Op:RsS:t:T:U:vwWxZ:",
long_options, &optindex)) != -1)
{
switch (c)
@@ -453,6 +461,11 @@ main(int argc, char **argv)
dopt.cparams.dbname = pg_strdup(optarg);
break;
+ case 'e': /* include extension(s) */
+simple_string_list_append(&extension_include_patterns, optarg);
+dopt.include_everything = false;
+break;
+
case 'E': /* Dump encoding */
dumpencoding = pg_strdup(optarg);
break;
@@ -832,6 +845,16 @@ main(int argc, char **argv)
/* non-matching e
Re: Extensions not dumped when --schema is used
Le sam. 20 févr. 2021 à 13:25, Michael Paquier a écrit : > On Thu, Feb 18, 2021 at 11:13:06AM +0100, Guillaume Lelarge wrote: > > I finally managed to get a working TAP test for my patch. I have no idea > if > > it's good, and if it's enough. Anyway, new version of the patch attached. > > As presented in this patch, specifying both --extension and > --table/--schema means that pg_dump will dump both tables and > extensions matching the pattern passed down. But shouldn't extensions > not be dumped if --table or --schema is used? Combining --schema with > --table implies that the schema part is ignored, for instance. > > Actually, that's the whole point of the patch. Imagine someone who wants to backup a table with a citext column. With the --table option, this user will only have the table, without the extension that would allow to restore the dump. He will have to remember to create the extension before. The new option allows him to add the CREATE EXTENSION he needs into his dump. The comment at the top of selectDumpableExtension() is incorrect, > missing the new case added by --extension. > > The use of strict_names looks right to me, but you have missed a > refresh of the documentation of --strict-names that applies to > --extension with this patch. > > + dump_cmd => [ > + 'pg_dump', "--file=$tempdir/test_schema_plus_extensions.sql", > + '--schema=dump_test', '--extension=plpgsql', '--no-sync', > What's the goal of this test. plpgsql is a system extension so it > will never be dumped. It seems to me that any tests related to this > patch should be added to src/test/modules/test_pg_dump/, which > includes a custom extension in the test, that could be dumped. > > +dumped. Multiple schemas can be selected by writing multiple > +-e switches. The s/schemas/extensions/. > You're right on all these points. I'm on vacation this week, but I'll build a v3 with these issues fixed when I'll get back from vacation. Thanks. -- Guillaume.
Re: Extensions not dumped when --schema is used
Le sam. 20 févr. 2021 à 17:31, Tom Lane a écrit : > Michael Paquier writes: > > As presented in this patch, specifying both --extension and > > --table/--schema means that pg_dump will dump both tables and > > extensions matching the pattern passed down. But shouldn't extensions > > not be dumped if --table or --schema is used? Combining --schema with > > --table implies that the schema part is ignored, for instance. > > I haven't read the patch, but the behavior I would expect is: > > 1. If --extension=pattern is given, then extensions matching the > pattern are included in the dump, regardless of other switches. > (Conversely, use of --extension doesn't affect choices about what > other objects are dumped.) > > 2. Without --extension, the behavior is backward compatible, > ie, dump extensions in an include_everything dump but not > otherwise. > > Yes, that's what it's supposed to do. Maybe we could have a separate discussion as to which switches turn > off include_everything, but that seems independent of this patch. > > +1 -- Guillaume.
Re: Extensions not dumped when --schema is used
Le lun. 15 mars 2021 à 06:00, Michael Paquier a écrit : > On Sun, Feb 21, 2021 at 08:14:45AM +0900, Michael Paquier wrote: > > Okay, that sounds fine to me. Thanks for confirming. > > Guillaume, it has been a couple of weeks since your last update. Are > you planning to send a new version of the patch? > This is on my TODO list, but I can't find the time right now. If someone's interested in this patch, i have no problem with him/her working on it. Otherwise, I'll do it as soon as possible (meaning probably in two weeks).
Re: Extensions not dumped when --schema is used
Le lun. 15 mars 2021 à 11:32, Julien Rouhaud a écrit : > Le lun. 15 mars 2021 à 18:25, Michael Paquier a > écrit : > >> On Mon, Mar 15, 2021 at 06:21:55PM +0800, Julien Rouhaud wrote: >> > Is that a feature you really want to see in pg14? If yes and if you're >> sure >> > you won't have time to work on the patch within 2 weeks I can take care >> of >> > addressing all comments. >> >> A lot of things will depend on the feature freeze date, but the sooner >> a version is available, the sooner I could look at what is proposed. >> > > I totally agree. > > Recalling my memories of the discussion, there was not much to >> address, and the logic was rather clear. >> > > yes, I just had a look it's only a matter of adapting the tests to > test_pg_dump and fixing a few typos AFAICS. > Jeez, you're answering too fast. I was working on my own answer when Julien sent another reply 😅 Anyways. Yeah, I know we're near feature freeze. This feature would be nice to have, but I don't feel strongly about it. I think this feature is currently lacking in PostgreSQL but I don't much care if it makes it to 14 or any future release. If you have time to work on the pg_dump test suite and are interested, then sure, go ahead, I'm fine with this. Otherwise I'll do it in a few weeks and if it means it'll land in v15, then be it. That's not an issue in itself. Though, I really appreciate the concern. Thanks. >
Re: Extensions not dumped when --schema is used
Le mer. 31 mars 2021 à 02:37, Michael Paquier a écrit : > On Tue, Mar 30, 2021 at 12:02:45PM +0900, Michael Paquier wrote: > > Okay. So I have looked at that stuff in details, and after fixing > > all the issues reported upthread in the code, docs and tests I am > > finishing with the attached. The tests have been moved out of > > src/bin/pg_dump/ to src/test/modules/test_pg_dump/, and include both > > positive and negative tests (used the trick with plpgsql for the > > latter to avoid the dump of the extension test_pg_dump or any data > > related to it). > > I have double-checked this stuff this morning, and did not notice any > issues. So, applied. > Thanks a lot. I've seen your email yesterday but had too much work going on to find the time to test your patch. Anyway, I'll take a look at how you coded the TAP test to better understand that part and to be able to do it myself next time. Thanks again.
Re: Extensions not dumped when --schema is used
Le jeu. 15 avr. 2021 à 09:58, Michael Paquier a écrit : > On Wed, Apr 14, 2021 at 05:31:15AM -0700, Noah Misch wrote: > > The name "without_extension_explicit_schema" arose because that test > differs > > from the "without_extension" test by adding --schema=public. The test > named > > "without_extension_implicit_schema" differs from "without_extension" by > adding > > --schema=regress_pg_dump_schema, so the word "implicit" feels > not-descriptive > > of the test. I recommend picking a different name. Other than that, the > > change looks good. > > Thanks for the review. I have picked up "internal" instead, as > that's the schema created within the extension itself, and applied the > patch. > Thanks for the work on this. I didn't understand everything on the issue, which is why I didn't say a thing, but I followed the thread, and very much appreciated the fix. -- Guillaume.
Lots of memory allocated when reassigning Large Objects
Hi, One of our customers had a very bad issue while trying to reassign objects from user A to user B. He had a lot of them, and the backend got very hungry for memory. It finally all went down when the linux kernel decided to kill the backend (-9 of course). I attach three shell scripts showing the issue. create.sh creates a database and two users. Then it imports a million Large Objects in this new database. There's no drop.sh as it is a simple "dropdb foodb". session1_monitor.sh will start logging memory usage in the server log file every second. So it needs v14, but our customer is in v11. While this script runs, you can start session2_reindex.sh. This script will only run a reassign from one user to another. Here is what I get in the server log file: $ grep "Grand total" 14.log LOG: Grand total: 15560832 bytes... LOG: Grand total: 68710528 bytes... LOG: Grand total: 119976064 bytes.. LOG: Grand total: 171626624 bytes... LOG: Grand total: 224211072 bytes... LOG: Grand total: 276615296 bytes... LOG: Grand total: 325611648 bytes... LOG: Grand total: 378196096 bytes... LOG: Grand total: 429838464 bytes... LOG: Grand total: 481104000 bytes... IOW, it's asking for at least 481MB to reassign 1 million empty LO. It strikes me as odd. FWIW, the biggest memory context is this one: LOG: level: 2; PortalContext: 479963904 total in 58590 blocks; 2662328 free (32567 chunks); 477301576 used: Memory is released at the end of the reassignment. So it's definitely not leaked forever, but only during the operation, which looks like a missing pfree (or something related). I've tried to find something like that in the code somewhere, but to no avail. I'm pretty sure I missed something, which is the reason for this email :) Thanks. Regards. PS : we've found a workaround to make it work for our customer (executing all the required ALTER LARGE OBJECT ... OWNER TO ...), but I'm still amazed by this weird behaviour. -- Guillaume. create.sh Description: application/shellscript session2_reindex.sh Description: application/shellscript session1_monitor.sh Description: application/shellscript
Re: Lots of memory allocated when reassigning Large Objects
Le lun. 29 nov. 2021 à 19:40, Tom Lane a écrit :
> Justin Pryzby writes:
> > I reproduced the issue like this.
>
> > psql postgres -c 'CREATE ROLE two WITH login superuser'
> > psql postgres two -c "SELECT lo_import('/dev/null') FROM
> generate_series(1,22111)" >/dev/null
> > psql postgres -c 'SET client_min_messages=debug; SET
> log_statement_stats=on;' -c 'begin; REASSIGN OWNED BY two TO pryzbyj;
> rollback;'
>
> Confirmed here, although I needed to use a lot more than 22K large objects
> to see a big leak.
>
>
So do I.
> I didn't find the root problem, but was able to avoid the issue by
> creating a
> > new mem context. I wonder if there are a bunch more issues like this.
>
> I poked into it with valgrind, and identified the major leak as being
> stuff that is allocated by ExecOpenIndices and not freed by
> ExecCloseIndices. The latter knows it's leaking:
>
> /*
> * XXX should free indexInfo array here too? Currently we assume
> that
> * such stuff will be cleaned up automatically in
> FreeExecutorState.
> */
>
> On the whole, I'd characterize this as DDL code using pieces of the
> executor without satisfying the executor's expectations as to environment
> --- specifically, that it'll be run in a memory context that doesn't
> outlive executor shutdown. Normally, any one DDL operation does a limited
> number of catalog updates so that small per-update leaks don't cost that
> much ... but REASSIGN OWNED creates a loop that can invoke ALTER OWNER
> many times.
>
> I think your idea of creating a short-lived context is about right.
> Another idea we could consider is to do that within CatalogTupleUpdate;
> but I'm not sure that the cost/benefit ratio would be good for most
> operations. Anyway I think ALTER OWNER has other leaks outside the
> index-update operations, so we'd still need to do this within
> REASSIGN OWNED's loop.
>
>
I've tried Justin's patch but it didn't help with my memory allocation
issue. FWIW, I attach the patch I used in v14.
DROP OWNED BY likely has similar issues.
>
>
Didn't try it, but it wouldn't be a surprise.
--
Guillaume.
diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index d89a9fa2bf..68dad27afb 100644
--- a/src/backend/catalog/pg_shdepend.c
+++ b/src/backend/catalog/pg_shdepend.c
@@ -65,6 +65,7 @@
#include "storage/lmgr.h"
#include "utils/acl.h"
#include "utils/fmgroids.h"
+#include "utils/memutils.h"
#include "utils/syscache.h"
typedef enum
@@ -1549,6 +1550,10 @@ shdepReassignOwned(List *roleids, Oid newrole)
while ((tuple = systable_getnext(scan)) != NULL)
{
Form_pg_shdepend sdepForm = (Form_pg_shdepend) GETSTRUCT(tuple);
+ MemoryContext cxt, oldcxt;
+
+ cxt = AllocSetContextCreate(CurrentMemoryContext, "shdepReassignOwned cxt", ALLOCSET_DEFAULT_SIZES);
+ oldcxt = MemoryContextSwitchTo(cxt);
/*
* We only operate on shared objects and objects in the current
@@ -1656,6 +1661,9 @@ shdepReassignOwned(List *roleids, Oid newrole)
}
/* Make sure the next iteration will see my changes */
CommandCounterIncrement();
+
+ MemoryContextSwitchTo(oldcxt);
+ MemoryContextDelete(cxt);
}
systable_endscan(scan);
Re: Lots of memory allocated when reassigning Large Objects
Le lun. 29 nov. 2021 à 20:39, Tom Lane a écrit : > Guillaume Lelarge writes: > > I've tried Justin's patch but it didn't help with my memory allocation > > issue. FWIW, I attach the patch I used in v14. > > [ looks closer ... ] Ah, that patch is a bit buggy: it fails to do the > right thing in the cases where the loop does a "continue". The attached > revision seems to behave properly. > > I still see a small leakage, which I think is due to accumulation of > pending sinval messages for the catalog updates. I'm curious whether > that's big enough to be a problem for Guillaume's use case. (We've > speculated before about bounding the memory used for pending sinval > in favor of just issuing a cache reset when the list would be too > big. But nobody's done anything about it, suggesting that people > seldom have a problem in practice.) > > I've tried your patch with my test case. It still uses a lot of memory. Actually even more. I have this with the log_statement_stats: 1185072 kB max resident size And I have this with the log-memory-contexts function: LOG: Grand total: 1007796352 bytes in 320 blocks; 3453512 free (627 chunks); 1004342840 used Contrary to Justin's patch, the shdepReassignOwned doesn't seem to be used. I don't get any shdepReassignOwned line in the log file. I tried multiple times to avoid any mistake on my part, but got same result. >> DROP OWNED BY likely has similar issues. > > > Didn't try it, but it wouldn't be a surprise. > > I tried just changing the REASSIGN to a DROP in Justin's example, > and immediately hit > > ERROR: out of shared memory > HINT: You might need to increase max_locks_per_transaction. > > thanks to the per-object locks we try to acquire. So I'm not > sure that the DROP case can reach an interesting amount of > local memory leaked before it runs out of lock-table space. > > I've hit the same issue when I tried my ALTER LARGE OBJECT workaround in one transaction. -- Guillaume.
Re: Lots of memory allocated when reassigning Large Objects
Le lun. 29 nov. 2021 à 22:27, Tom Lane a écrit : > Guillaume Lelarge writes: > > Le lun. 29 nov. 2021 à 20:39, Tom Lane a écrit : > >> [ looks closer ... ] Ah, that patch is a bit buggy: it fails to do the > >> right thing in the cases where the loop does a "continue". The attached > >> revision seems to behave properly. > > > I've tried your patch with my test case. It still uses a lot of memory. > > Actually even more. > > Hmm ... I tried it with your test case, and I see the backend completing > the query without going beyond 190MB used (which is mostly shared memory). > Without the patch it blows past that point very quickly indeed. > > I'm checking it in HEAD though; perhaps there's something else wrong > in the back branches? > > That's also what I was thinking. I was only trying with v14. I just checked with v15devel, and your patch works alright. So there must be something else with back branches. -- Guillaume.
Re: Lots of memory allocated when reassigning Large Objects
Le lun. 29 nov. 2021 à 22:47, Tom Lane a écrit : > Guillaume Lelarge writes: > > Le lun. 29 nov. 2021 à 22:27, Tom Lane a écrit : > >> I'm checking it in HEAD though; perhaps there's something else wrong > >> in the back branches? > > > That's also what I was thinking. I was only trying with v14. I just > checked > > with v15devel, and your patch works alright. So there must be something > > else with back branches. > > Thanks for confirming, I'll dig into it later. > > Thanks a lot. -- Guillaume.
Re: Lots of memory allocated when reassigning Large Objects
Le mar. 30 nov. 2021 à 00:25, Tom Lane a écrit : > Guillaume Lelarge writes: > > Le lun. 29 nov. 2021 à 22:27, Tom Lane a écrit : > >> I'm checking it in HEAD though; perhaps there's something else wrong > >> in the back branches? > > > That's also what I was thinking. I was only trying with v14. I just > checked > > with v15devel, and your patch works alright. So there must be something > > else with back branches. > > AFAICT the patch fixes what it intends to fix in v14 too. The reason the > residual leak is worse in v14 is that the sinval message queue is bulkier. > We improved that in HEAD in commit 3aafc030a. I wanted to make sure that commit 3aafc030a fixed this issue. So I did a few tests: without 3aafc030a, without your latest patch 1182148 kB max resident size with 3aafc030a, without your latest patch 1306812 kB max resident size without 3aafc030a, with your latest patch 1182128 kB max resident size with 3aafc030a, with your latest patch 180996 kB max resident size Definitely, 3aafc030a and your latest patch allow PostgreSQL to use much less memory. Going from 1GB to 180MB is awesome. I tried to cherry-pick 3aafc030a on v14, but it didn't apply cleanly, and the work was a bit overwhelming for me, at least in the morning. I'll try again today, but I don't have much hope. I'm not sure if I want to > take the risk of back-patching that, even now that it's aged a couple > months in the tree. It is a pretty localized fix, but it makes some > assumptions about usage patterns that might not hold up. > > I understand. Of course, it would be better if it could be fixed for each supported version but I'm already happy that it could be fixed in the next release. -- Guillaume.
Re: Lots of memory allocated when reassigning Large Objects
Le mer. 1 déc. 2021 à 19:48, Tom Lane a écrit : > Justin Pryzby writes: > > @Guillaume: Even if memory use with the patch isn't constant, I imagine > it's > > enough to have avoided OOM. > > I think it's good enough in HEAD. In the back branches, the sinval > queue growth is bad enough that there's still an issue. Still, > this is a useful improvement, so I added some comments and pushed it. > > Thanks. -- Guillaume.
Probable memory leak with ECPG and AIX
Hello, Our customer thinks he has found a memory leak on ECPG and AIX. The code is quite simple. It declares a cursor, opens it, and fetches the only line available in the table many times. After some time, the client crashes with a segfault error. According to him, it consumed around 256MB. What's weird is that it works great on Linux, but crashed on AIX. One coworker thought it could be the compiler. Our customer used cc, but he also tried with gcc, and got the same error. The test case is attached (testcase.pgc is the ECPG code, testcase.sh is what our customer used to precompile and compile his code). Do you have any idea why that happens on AIX? Two queries to create the table and populate it with a single record: CREATE TABLE foo( key integer PRIMARY KEY, value character varying(20) ); INSERT INTO foo values (1, 'one'); Thanks. Regards. -- Guillaume. testcase.pgc Description: Binary data testcase.sh Description: application/shellscript
Re: Probable memory leak with ECPG and AIX
Le sam. 11 déc. 2021 à 07:52, Justin Pryzby a écrit : > On Fri, Dec 10, 2021 at 03:40:50PM +0100, Guillaume Lelarge wrote: > > Hello, > > > > Our customer thinks he has found a memory leak on ECPG and AIX. > > > > The code is quite simple. It declares a cursor, opens it, and fetches the > > only line available in the table many times. After some time, the client > > crashes with a segfault error. According to him, it consumed around > 256MB. > > What's weird is that it works great on Linux, but crashed on AIX. One > > coworker thought it could be the compiler. Our customer used cc, but he > > also tried with gcc, and got the same error. > > A memory leak isn't the same as a segfault (although I don't know how AIX > responds to OOM). > > Can you show that it's a memory leak ? Show RAM use increasing > continuously > and linearly with loop count. > > How many loops does it take to crash ? > > Could you obtain a backtrace ? > > Thanks. I'll try to get all these informations, but it won't be before monday. -- Guillaume.
Autovacuum and idle_session_timeout
Hello, I've been reading the autovacuum code (the launcher and the worker) on the 14 branch. As previously, I've seen some configuration at the beginning, especially for statement_timeout, lock_timeout and idle_in_transaction_session_timeout, and I was surprised to discover there was no configuration for idle_session_timeout. I'm not sure the code should set it to 0 as well (otherwise I'd have written a patch), but, if there was a decision made to ignore its value, I'd be interested to know the reason. I could guess for the autovacuum worker (it seems to work in a transaction, so it's already handled by the idle_in_transaction_timeout), but I have no idea for the autovacuum launcher. If it was just missed, I could write a patch this week to fix this. Regards. -- Guillaume.
Re: Autovacuum and idle_session_timeout
Le jeu. 30 déc. 2021 à 11:44, Japin Li a écrit : > > On Thu, 30 Dec 2021 at 17:18, Guillaume Lelarge > wrote: > > Hello, > > > > I've been reading the autovacuum code (the launcher and the worker) on > the > > 14 branch. As previously, I've seen some configuration at the beginning, > > especially for statement_timeout, lock_timeout and > > idle_in_transaction_session_timeout, and I was surprised to discover > there > > was no configuration for idle_session_timeout. I'm not sure the code > should > > set it to 0 as well (otherwise I'd have written a patch), but, if there > was > > a decision made to ignore its value, I'd be interested to know the > reason. > > I could guess for the autovacuum worker (it seems to work in a > transaction, > > so it's already handled by the idle_in_transaction_timeout), but I have > no > > idea for the autovacuum launcher. > > > > If it was just missed, I could write a patch this week to fix this. > > > > Oh, it was just missed. I didn't note set autovacuum code set those > settings, > I think we should also set idle_session_timeout to 0. > > Should we also change this for pg_dump and pg_backup_archiver? > > pg_dump works in a single transaction, so it's already dealt with idle_in_transaction_timeout. Though I guess setting both would work too. -- Guillaume.
Re: Autovacuum and idle_session_timeout
Le jeu. 30 déc. 2021 à 12:01, Japin Li a écrit : > > On Thu, 30 Dec 2021 at 18:53, Guillaume Lelarge > wrote: > > Le jeu. 30 déc. 2021 à 11:44, Japin Li a écrit : > > > >> > > pg_dump works in a single transaction, so it's already dealt with > > idle_in_transaction_timeout. Though I guess setting both would work too. > > Attached fix this, please consider reveiew it. Thanks. > > I've read it and it really looks like what I would have done. Sounds good to me. -- Guillaume.
Re: Autovacuum and idle_session_timeout
Le jeu. 30 déc. 2021 à 17:25, Tom Lane a écrit : > Japin Li writes: > > On Thu, 30 Dec 2021 at 18:53, Guillaume Lelarge > wrote: > >> pg_dump works in a single transaction, so it's already dealt with > >> idle_in_transaction_timeout. Though I guess setting both would work too. > > > Attached fix this, please consider reveiew it. Thanks. > > This seems rather pointless to me. The idle-session timeout is only > activated in PostgresMain's input loop, so it will never be reached > in autovacuum or other background workers. (The same is true for > idle_in_transaction_session_timeout, so the fact that somebody made > autovacuum.c clear that looks like cargo-cult programming from here, > not useful code.) And as for pg_dump, how would it ever trigger the > timeout? It's not going to sit there thinking, especially not > outside a transaction. > > Agreed. It makes more sense. So no need for the patch. Thanks to both. -- Guillaume.
Re: Extensions not dumped when --schema is used
Hi,
Le sam. 23 mai 2020 à 14:53, Guillaume Lelarge a
écrit :
> Le mer. 20 mai 2020 à 16:39, Tom Lane a écrit :
>
>> Guillaume Lelarge writes:
>> > Le mer. 20 mai 2020 à 11:26, Daniel Gustafsson a
>> écrit :
>> >> The question is what --extensions should do: only dump any
>> >> extensions that objects in the schema depend on; require a pattern and
>> only
>> >> dump matching extensions; dump all extensions (probably not) or
>> something
>> >> else?
>>
>> > Actually, "dump all extensions" (#3) would make sense to me, and has my
>> > vote.
>>
>> I think that makes no sense at all. By definition, a dump that's been
>> restricted with --schema, --table, or any similar switch is incomplete
>> and may not restore on its own. Typical examples include foreign key
>> references to tables in other schemas, views using functions in other
>> schemas, etc etc. I see no reason for extension dependencies to be
>> treated differently from those cases.
>>
>>
> Agreed.
>
> In any use of selective dump, it's the user's job to select a set of
>> objects that she wants dumped (or restored). Trying to second-guess that
>> is mostly going to make the feature less usable for power-user cases.
>>
>>
> Agreed, though right now he has no way to do this for extensions.
>
> As a counterexample, what if you want the dump to be restorable on a
>> system that doesn't have all of the extensions available on the source?
>> You carefully pick out the tables that you need, which don't require the
>> unavailable extensions ... and then pg_dump decides you don't know what
>> you're doing and includes all the problematic extensions anyway.
>>
>>
> That's true.
>
> I could get behind an "--extensions=PATTERN" switch to allow selective
>> addition of extensions to a selective dump, but I don't want to see us
>> overruling the user's choices about what to dump.
>>
>>
> With all your comments, I can only agree to your views. I'll try to work
> on this anytime soon.
>
>
"Anytime soon" was a long long time ago, and I eventually completely forgot
this, sorry. As nobody worked on it yet, I took a shot at it. See attached
patch.
I don't know if I should add this right away in the commit fest app. If
yes, I guess it should go on the next commit fest (2021-03), right?
--
Guillaume.
commit 97c41b05b4aa20f4187aedbed79d67874d2cbbbd
Author: Guillaume Lelarge
Date: Mon Jan 25 14:25:53 2021 +0100
Add a --extension flag to dump specific extensions
Version 1.
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index bcbb7a25fb..95d45fabfb 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -215,6 +215,38 @@ PostgreSQL documentation
+
+ -e pattern
+ --extension=pattern
+
+
+Dump only extensions matching pattern. When this option is not
+specified, all non-system extensions in the target database will be
+dumped. Multiple schemas can be selected by writing multiple
+-e switches. The pattern parameter is interpreted as a
+pattern according to the same rules used by
+psql's \d commands (see
+), so multiple extensions can also
+be selected by writing wildcard characters in the pattern. When using
+wildcards, be careful to quote the pattern if needed to prevent the
+shell from expanding the wildcards.
+
+
+
+
+ When -e is specified,
+ pg_dump makes no attempt to dump any other
+ database objects that the selected extension(s) might depend upon.
+ Therefore, there is no guarantee that the results of a
+ specific-extension dump can be successfully restored by themselves
+ into a clean database.
+
+
+
+
+
-E encoding
--encoding=encoding
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 798d14580e..3c1203e85c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -123,6 +123,9 @@ static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
static SimpleStringList foreign_servers_include_patterns = {NULL, NULL};
static SimpleOidList foreign_servers_include_oids = {NULL, NULL};
+static SimpleStringList extension_include_patterns = {NULL, NULL};
+static SimpleOidList extension_include_oids = {NULL, NULL};
+
static const CatalogId nilCatalogId = {0, 0};
/* override for standard extra_float_digits setting */
@@ -151,6 +154,10 @@ static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
Re: Extensions not dumped when --schema is used
Le mar. 26 janv. 2021 à 05:10, Julien Rouhaud a écrit : > On Mon, Jan 25, 2021 at 9:34 PM Guillaume Lelarge > wrote: > > > > "Anytime soon" was a long long time ago, and I eventually completely > forgot this, sorry. As nobody worked on it yet, I took a shot at it. See > attached patch. > > Great! > > I didn't reviewed it thoroughly yet, but after a quick look it sounds > sensible. I'd prefer to see some tests added, and it looks like a > test for plpgsql could be added quite easily. > > I tried that all afternoon yesterday, but failed to do so. My had still hurts, but I'll try again though it may take some time. > I don't know if I should add this right away in the commit fest app. If > yes, I guess it should go on the next commit fest (2021-03), right? > > Correct, and please add it on the commit fest! > Done, see https://commitfest.postgresql.org/32/2956/. -- Guillaume.
Re: Extensions not dumped when --schema is used
Le mar. 26 janv. 2021 à 13:41, Guillaume Lelarge a écrit : > Le mar. 26 janv. 2021 à 05:10, Julien Rouhaud a > écrit : > >> On Mon, Jan 25, 2021 at 9:34 PM Guillaume Lelarge >> wrote: >> > >> > "Anytime soon" was a long long time ago, and I eventually completely >> forgot this, sorry. As nobody worked on it yet, I took a shot at it. See >> attached patch. >> >> Great! >> >> I didn't reviewed it thoroughly yet, but after a quick look it sounds >> sensible. I'd prefer to see some tests added, and it looks like a >> test for plpgsql could be added quite easily. >> >> > I tried that all afternoon yesterday, but failed to do so. My had still > hurts, but I'll try again though it may take some time. > > s/My had/My head/ .. > I don't know if I should add this right away in the commit fest app. If >> yes, I guess it should go on the next commit fest (2021-03), right? >> >> Correct, and please add it on the commit fest! >> > > Done, see https://commitfest.postgresql.org/32/2956/. > > -- Guillaume.
Re: Extensions not dumped when --schema is used
Hi, Thanks for the review. Le mer. 3 févr. 2021 à 18:33, Asif Rehman a écrit : > The following review has been posted through the commitfest application: > make installcheck-world: tested, passed > Implements feature: tested, passed > Spec compliant: not tested > Documentation:not tested > > The patch applies cleanly and looks fine to me. However consider this > scenario. > > - CREATE SCHEMA foo; > - CREATE EXTENSION file_fdw WITH SCHEMA foo; > - pg_dump --file=/tmp/test.sql --exclude-schema=foo postgres > > This will still include the extension 'file_fdw' in the backup script. > Shouldn't it be excluded as well? > > The new status of this patch is: Waiting on Author > This behaviour is already there without my patch, and I think it's a valid behaviour. An extension doesn't belong to a schema. Its objects do, but the extension doesn't. -- Guillaume.
"Multiple table synchronizations are processed serially" still happens
Hi,
One of my customers has an issue with logical replication. As $SUBJECT
says, multiple table synchronization happens serially. To be honest, it
doesn't do this every time. It happens when the tables are big enough.
This issue was already described on this thread (from 2017):
https://www.postgresql.org/message-id/flat/CAD21AoC2KJdavS7MFffmSsRc1dn3Vg_0xmuc=upbrz-_mux...@mail.gmail.com
This thread was closed by a commit (
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=6c2003f8a1bbc7c192a2e83ec51581c018aa162f)
which apparently fixed the issue for the OP.
Attached is a small test case where it still happens for me on 12.6, 11.11,
and 10.16. I can't make it happen on 13.2. I don't know why. It may imply
bigger tables for 13.2, but why? I simply don't know.
Anyway, the issue at the end of the test case is that synchronizations
sometimes happen serially. You can see on the process list that one
walsender process is waiting in "idle in transaction" state:
guillau+ 4868222227 0 10:44 ?00:00:00
/opt/postgresql/12/bin/postgres
guillau+ 486824 486822 0 10:44 ?00:00:01 postgres: testcase:
checkpointer
guillau+ 486825 486822 0 10:44 ?00:00:04 postgres: testcase:
background writer
guillau+ 486826 486822 1 10:44 ?00:00:06 postgres: testcase:
walwriter
guillau+ 486827 486822 0 10:44 ?00:00:00 postgres: testcase:
autovacuum launcher
guillau+ 486828 486822 0 10:44 ?00:00:00 postgres: testcase:
stats collector
guillau+ 486829 486822 0 10:44 ?00:00:00 postgres: testcase:
logical replication launcher
guillau+ 489822 486822 0 10:55 ?00:00:00 postgres: testcase:
logical replication worker for subscription 16436
guillau+ 489824 486822 10 10:55 ?00:00:01 postgres: testcase:
walsender repuser ::1(38770) idle
guillau+ 489825 486822 22 10:55 ?00:00:02 postgres: testcase:
logical replication worker for subscription 16436 sync 16416
guillau+ 489826 486822 8 10:55 ?00:00:00 postgres: testcase:
walsender repuser ::1(38772) COPY
guillau+ 489827 486822 0 10:55 ?00:00:00 postgres: testcase:
logical replication worker for subscription 16436 sync 16427
guillau+ 489828 486822 0 10:55 ?00:00:00 postgres: testcase:
walsender repuser ::1(38774) idle in transaction waiting
And the log says (from the start of the subscription):
2021-04-28 10:55:32.337 CEST [489822] LOG: logical replication apply
worker for subscription "sub" has started
2021-04-28 10:55:32.342 CEST [489824] LOG: duration: 0.426 ms statement:
SELECT pg_catalog.set_config('search_path', '', false);
2021-04-28 10:55:32.342 CEST [489824] LOG: received replication command:
IDENTIFY_SYSTEM
2021-04-28 10:55:32.342 CEST [489824] LOG: received replication command:
START_REPLICATION SLOT "sub" LOGICAL 0/0 (proto_version '1',
publication_names '"pub"')
2021-04-28 10:55:32.342 CEST [489824] LOG: starting logical decoding for
slot "sub"
2021-04-28 10:55:32.342 CEST [489824] DETAIL: Streaming transactions
committing after 1/FF5D8130, reading WAL from 1/FF5D80F8.
2021-04-28 10:55:32.342 CEST [489824] LOG: logical decoding found
consistent point at 1/FF5D80F8
2021-04-28 10:55:32.342 CEST [489824] DETAIL: There are no running
transactions.
2021-04-28 10:55:32.345 CEST [489825] LOG: logical replication table
synchronization worker for subscription "sub", table "foo" has started
2021-04-28 10:55:32.348 CEST [489826] LOG: duration: 0.315 ms statement:
SELECT pg_catalog.set_config('search_path', '', false);
2021-04-28 10:55:32.349 CEST [489826] LOG: duration: 0.041 ms statement:
BEGIN READ ONLY ISOLATION LEVEL REPEATABLE READ
2021-04-28 10:55:32.349 CEST [489826] LOG: received replication command:
CREATE_REPLICATION_SLOT "sub_16436_sync_16416" TEMPORARY LOGICAL pgoutput
USE_SNAPSHOT
2021-04-28 10:55:32.355 CEST [489827] LOG: logical replication table
synchronization worker for subscription "sub", table "bar" has started
2021-04-28 10:55:32.359 CEST [489828] LOG: duration: 0.431 ms statement:
SELECT pg_catalog.set_config('search_path', '', false);
2021-04-28 10:55:32.359 CEST [489828] LOG: duration: 0.048 ms statement:
BEGIN READ ONLY ISOLATION LEVEL REPEATABLE READ
2021-04-28 10:55:32.360 CEST [489828] LOG: received replication command:
CREATE_REPLICATION_SLOT "sub_16436_sync_16427" TEMPORARY LOGICAL pgoutput
USE_SNAPSHOT
2021-04-28 10:55:32.407 CEST [489826] LOG: logical decoding found
consistent point at 1/FF602880
2021-04-28 10:55:32.407 CEST [489826] DETAIL: There are no running
transactions.
2021-04-28 10:55:32.409 CEST [489826] LOG: duration: 1.262 ms statement:
SELECT c.oid, c.relreplident FROM pg_catalog.pg_class c INNER JOIN
pg_catalog.pg_namespace nON (c.relnamespace = n.oid) WHERE
n.nspname = 's01' AND c.relname = 'foo' AND c.relkind = 'r'
2021-04-28 10:55:32.410 CEST [489826] LOG: duration: 1.347 ms statement:
SELECT a.attname, a.atttypid, a.atttypmod, a.a
Re: "Multiple table synchronizations are processed serially" still happens
Le mer. 28 avr. 2021 à 11:12, Guillaume Lelarge a écrit : > Hi, > > One of my customers has an issue with logical replication. As $SUBJECT > says, multiple table synchronization happens serially. To be honest, it > doesn't do this every time. It happens when the tables are big enough. > > This issue was already described on this thread (from 2017): > https://www.postgresql.org/message-id/flat/CAD21AoC2KJdavS7MFffmSsRc1dn3Vg_0xmuc=upbrz-_mux...@mail.gmail.com > > This thread was closed by a commit ( > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=6c2003f8a1bbc7c192a2e83ec51581c018aa162f) > which apparently fixed the issue for the OP. > > Attached is a small test case where it still happens for me on 12.6, > 11.11, and 10.16. I can't make it happen on 13.2. I don't know why. It may > imply bigger tables for 13.2, but why? I simply don't know. > > Actually, it also happens on 13.2. -- Guillaume.
Re: "Multiple table synchronizations are processed serially" still happens
Le mer. 28 avr. 2021 à 11:37, Guillaume Lelarge a écrit : > Le mer. 28 avr. 2021 à 11:12, Guillaume Lelarge > a écrit : > >> Hi, >> >> One of my customers has an issue with logical replication. As $SUBJECT >> says, multiple table synchronization happens serially. To be honest, it >> doesn't do this every time. It happens when the tables are big enough. >> >> This issue was already described on this thread (from 2017): >> https://www.postgresql.org/message-id/flat/CAD21AoC2KJdavS7MFffmSsRc1dn3Vg_0xmuc=upbrz-_mux...@mail.gmail.com >> >> This thread was closed by a commit ( >> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=6c2003f8a1bbc7c192a2e83ec51581c018aa162f) >> which apparently fixed the issue for the OP. >> >> Attached is a small test case where it still happens for me on 12.6, >> 11.11, and 10.16. I can't make it happen on 13.2. I don't know why. It may >> imply bigger tables for 13.2, but why? I simply don't know. >> >> > Actually, it also happens on 13.2. > > Ping? :) It's been two weeks and no answer as far as I can tell. I may have missed something on this issue but I don't see what. It's not a big deal, but I would rather understand what's going on. Thanks. -- Guillaume.
Re: "Multiple table synchronizations are processed serially" still happens
Hi, Le jeu. 20 mai 2021 à 12:09, Amit Kapila a écrit : > On Wed, Apr 28, 2021 at 2:43 PM Guillaume Lelarge > wrote: > > > > And it logs the "still waiting" message as long as the first table is > being synchronized. Once this is done, it releases the lock, and the > synchronization of the second table starts. > > > > Is there something I didn't understand on the previous thread? > > > > It seems from a script that you are creating a subscription on the > same node as publication though in a different DB, right? Yes, that's right. If so, the > problem might be that copying the data of the first table creates a > transaction which blocks creation of the slot for second table copy. > I don't understand how a transaction could block the creation of a slot. Could you explain that to me? or do you know where this is explained in the documentation? The commit you referred will just fix the problem while reading the > data from the publisher not while writing data in the table in the > subscriber. > > > I'd like to know why serial synchronization happens, and if there's a > way to avoid it. > > > > I guess you need to create a subscription on a different node. > > Thanks. -- Guillaume.
Re: "Multiple table synchronizations are processed serially" still happens
Le ven. 21 mai 2021 à 05:43, Amit Kapila a écrit : > On Fri, May 21, 2021 at 1:30 AM Guillaume Lelarge > wrote: > > > > > >> If so, the > >> problem might be that copying the data of the first table creates a > >> transaction which blocks creation of the slot for second table copy. > > > > > > I don't understand how a transaction could block the creation of a slot. > Could you explain that to me? > > > > During the creation of the slot During the creation of the slot or during the creation of the subscription? because, in my tests, I create the slot before creating the snapshot. > , we need to build the initial snapshot > which is used for decoding WAL. Now, to build the initial snapshot, we > wait for all running xacts to finish. See functions > CreateReplicationSlot() and SnapBuildFindSnapshot(). > > If we have two workers, both will have a snapshot? they don't share the same snapshot? And if all this is true, I don't see how it could work when the replication happens between two clusters, and couldn't work when it happens with only one cluster. -- Guillaume.
Issue on catalogs.sgml
Hey, While working on the french translation of the manual, I found that one column of pg_stats_ext was on the pg_stats columns' list. Here is a quick patch to fix this. Regards. -- Guillaume. diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 8aebc4d12f..1063168646 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -12751,19 +12751,10 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx (references pg_attribute.attname) - Names of the columns included in the extended statistics object + Name of the column described by this row - - - exprs text[] - - - Expressions included in the extended statistics object - - - inherited bool @@ -13007,6 +12998,15 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx + + + exprs text[] + + + Expressions included in the extended statistics object + + + kinds char[]
Re: Issue on catalogs.sgml
Le mar. 25 mai 2021 à 00:05, Tom Lane a écrit : > Guillaume Lelarge writes: > > While working on the french translation of the manual, I found that one > > column of pg_stats_ext was on the pg_stats columns' list. Here is a quick > > patch to fix this. > > Right you are, and after casting a suspicious eye on the responsible > commit, I found another similar error. "patch" with the default > amount of context is not too bright about handling our documentation > tables :-(. > > Pushed. > > Thanks. -- Guillaume.
Re: Expose lock group leader pid in pg_stat_activity
Le mer. 25 déc. 2019 à 19:30, Julien Rouhaud a écrit :
> On Wed, Dec 25, 2019 at 7:03 PM Julien Rouhaud wrote:
> >
> > Guillaume (in Cc) recently pointed out [1] that it's currently not
> > possible to retrieve the list of parallel workers for a given backend
> > at the SQL level. His use case was to develop a function in plpgsql
> > to sample a given query wait event, but it's not hard to imagine other
> > useful use cases for this information, for instance doing some
> > analysis on the average number of workers per parallel query, or ratio
> > of parallel queries. IIUC parallel queries is for now the only user
> > of lock group, so this should work just fine.
> >
> > I'm attaching a trivial patch to expose the group leader pid if any
> > in pg_stat_activity. Quick example of usage:
> >
> > =# SELECT query, leader_pid,
> > array_agg(pid) filter(WHERE leader_pid != pid) AS members
> > FROM pg_stat_activity
> > WHERE leader_pid IS NOT NULL
> > GROUP BY query, leader_pid;
> >query | leader_pid |members
> > ---++---
> > select * from t1; | 28701 | {28728,28732}
> > (1 row)
> >
> >
> > [1] https://twitter.com/g_lelarge/status/1209486212190343168
>
> And I just realized that I forgot to update rule.out, sorry about
> that. v2 attached.
>
So I tried your patch this morning, and it works really well.
On a SELECT count(*), I got this:
SELECT leader_pid, pid, wait_event_type, wait_event, state, backend_type
FROM pg_stat_activity WHERE pid=111439 or leader_pid=111439;
┌┬┬─┬──┬┬─┐
│ leader_pid │ pid │ wait_event_type │ wait_event │ state │
backend_type │
├┼┼─┼──┼┼─┤
│ 111439 │ 111439 │ LWLock │ WALWriteLock │ active │ client
backend │
│ 111439 │ 116887 │ LWLock │ WALWriteLock │ active │ parallel
worker │
│ 111439 │ 116888 │ IO │ WALSync │ active │ parallel
worker │
└┴┴─┴──┴┴─┘
(3 rows)
and this from a CREATE INDEX:
┌┬┬─┬┬┬─┐
│ leader_pid │ pid │ wait_event_type │ wait_event │ state │
backend_type │
├┼┼─┼┼┼─┤
│ 111439 │ 111439 │ ││ active │ client
backend │
│ 111439 │ 118775 │ ││ active │ parallel
worker │
└┴┴─┴┴┴─┘
(2 rows)
Anyway, it applies cleanly, it compiles, and it works. Documentation is
available. So it looks to me it's good to go :)
--
Guillaume.
Re: Expose lock group leader pid in pg_stat_activity
Le jeu. 26 déc. 2019 à 09:49, Julien Rouhaud a écrit :
> On Thu, Dec 26, 2019 at 9:08 AM Guillaume Lelarge
> wrote:
> >
> > Le mer. 25 déc. 2019 à 19:30, Julien Rouhaud a
> écrit :
> >>
> >> On Wed, Dec 25, 2019 at 7:03 PM Julien Rouhaud
> wrote:
> >> >
> >> > Guillaume (in Cc) recently pointed out [1] that it's currently not
> >> > possible to retrieve the list of parallel workers for a given backend
> >> > at the SQL level. His use case was to develop a function in plpgsql
> >> > to sample a given query wait event, but it's not hard to imagine other
> >> > useful use cases for this information, for instance doing some
> >> > analysis on the average number of workers per parallel query, or ratio
> >> > of parallel queries. IIUC parallel queries is for now the only user
> >> > of lock group, so this should work just fine.
> >> >
> >> > I'm attaching a trivial patch to expose the group leader pid if any
> >> > in pg_stat_activity. Quick example of usage:
> >> >
> >> > =# SELECT query, leader_pid,
> >> > array_agg(pid) filter(WHERE leader_pid != pid) AS members
> >> > FROM pg_stat_activity
> >> > WHERE leader_pid IS NOT NULL
> >> > GROUP BY query, leader_pid;
> >> >query | leader_pid |members
> >> > ---++---
> >> > select * from t1; | 28701 | {28728,28732}
> >> > (1 row)
> >> >
> >> >
> >> > [1] https://twitter.com/g_lelarge/status/1209486212190343168
> >>
> >> And I just realized that I forgot to update rule.out, sorry about
> >> that. v2 attached.
> >
> >
> > So I tried your patch this morning, and it works really well.
> >
> > On a SELECT count(*), I got this:
> >
> > SELECT leader_pid, pid, wait_event_type, wait_event, state, backend_type
> FROM pg_stat_activity WHERE pid=111439 or leader_pid=111439;
> >
> >
> ┌┬┬─┬──┬┬─┐
> > │ leader_pid │ pid │ wait_event_type │ wait_event │ state │
> backend_type │
> >
> ├┼┼─┼──┼┼─┤
> > │ 111439 │ 111439 │ LWLock │ WALWriteLock │ active │ client
> backend │
> > │ 111439 │ 116887 │ LWLock │ WALWriteLock │ active │
> parallel worker │
> > │ 111439 │ 116888 │ IO │ WALSync │ active │
> parallel worker │
> >
> └┴┴─┴──┴┴─┘
> > (3 rows)
> >
> > and this from a CREATE INDEX:
> >
> >
> ┌┬┬─┬┬┬─┐
> > │ leader_pid │ pid │ wait_event_type │ wait_event │ state │
> backend_type │
> >
> ├┼┼─┼┼┼─┤
> > │ 111439 │ 111439 │ ││ active │ client
> backend │
> > │ 111439 │ 118775 │ ││ active │ parallel
> worker │
> >
> └┴┴─┴┴┴─┘
> > (2 rows)
> >
> > Anyway, it applies cleanly, it compiles, and it works. Documentation is
> available. So it looks to me it's good to go :)
>
> Thanks for the review Guillaume. Double checking the doc, I see that
> I made a copy/pasto mistake in the new field name. Attached v3 should
> be all good.
>
Feeling bad I missed this. But, yeah, it's much better with the right
column's name.
For me, it's looking good to be ready for commiter. Should I set it this
way in the Commit Fest app?
--
Guillaume.
Re: Expose lock group leader pid in pg_stat_activity
Le jeu. 26 déc. 2019 à 10:26, Julien Rouhaud a écrit :
> On Thu, Dec 26, 2019 at 10:20 AM Guillaume Lelarge
> wrote:
> >
> > Le jeu. 26 déc. 2019 à 09:49, Julien Rouhaud a
> écrit :
> >>
> >> On Thu, Dec 26, 2019 at 9:08 AM Guillaume Lelarge
> >> wrote:
> >> >
> >> > Le mer. 25 déc. 2019 à 19:30, Julien Rouhaud a
> écrit :
> >> >>
> >> >> On Wed, Dec 25, 2019 at 7:03 PM Julien Rouhaud
> wrote:
> >> >> >
> >> >> > Guillaume (in Cc) recently pointed out [1] that it's currently not
> >> >> > possible to retrieve the list of parallel workers for a given
> backend
> >> >> > at the SQL level. His use case was to develop a function in
> plpgsql
> >> >> > to sample a given query wait event, but it's not hard to imagine
> other
> >> >> > useful use cases for this information, for instance doing some
> >> >> > analysis on the average number of workers per parallel query, or
> ratio
> >> >> > of parallel queries. IIUC parallel queries is for now the only
> user
> >> >> > of lock group, so this should work just fine.
> >> >> >
> >> >> > I'm attaching a trivial patch to expose the group leader pid if any
> >> >> > in pg_stat_activity. Quick example of usage:
> >> >> >
> >> >> > =# SELECT query, leader_pid,
> >> >> > array_agg(pid) filter(WHERE leader_pid != pid) AS members
> >> >> > FROM pg_stat_activity
> >> >> > WHERE leader_pid IS NOT NULL
> >> >> > GROUP BY query, leader_pid;
> >> >> >query | leader_pid |members
> >> >> > ---++---
> >> >> > select * from t1; | 28701 | {28728,28732}
> >> >> > (1 row)
> >> >> >
> >> >> >
> >> >> > [1] https://twitter.com/g_lelarge/status/1209486212190343168
> >> >>
> >> >> And I just realized that I forgot to update rule.out, sorry about
> >> >> that. v2 attached.
> >> >
> >> >
> >> > So I tried your patch this morning, and it works really well.
> >> >
> >> > On a SELECT count(*), I got this:
> >> >
> >> > SELECT leader_pid, pid, wait_event_type, wait_event, state,
> backend_type FROM pg_stat_activity WHERE pid=111439 or leader_pid=111439;
> >> >
> >> >
> ┌┬┬─┬──┬┬─┐
> >> > │ leader_pid │ pid │ wait_event_type │ wait_event │ state │
> backend_type │
> >> >
> ├┼┼─┼──┼┼─┤
> >> > │ 111439 │ 111439 │ LWLock │ WALWriteLock │ active │
> client backend │
> >> > │ 111439 │ 116887 │ LWLock │ WALWriteLock │ active │
> parallel worker │
> >> > │ 111439 │ 116888 │ IO │ WALSync │ active │
> parallel worker │
> >> >
> └┴┴─┴──┴┴─┘
> >> > (3 rows)
> >> >
> >> > and this from a CREATE INDEX:
> >> >
> >> >
> ┌┬┬─┬┬┬─┐
> >> > │ leader_pid │ pid │ wait_event_type │ wait_event │ state │
> backend_type │
> >> >
> ├┼┼─┼┼┼─┤
> >> > │ 111439 │ 111439 │ ││ active │
> client backend │
> >> > │ 111439 │ 118775 │ ││ active │
> parallel worker │
> >> >
> └┴┴─┴┴┴─┘
> >> > (2 rows)
> >> >
> >> > Anyway, it applies cleanly, it compiles, and it works. Documentation
> is available. So it looks to me it's good to go :)
> >>
> >> Thanks for the review Guillaume. Double checking the doc, I see that
> >> I made a copy/pasto mistake in the new field name. Attached v3 should
> >> be all good.
> >
> >
> > Feeling bad I missed this. But, yeah, it's much better with the right
> column's name.
> >
> > For me, it's looking good to be ready for commiter. Should I set it this
> way in the Commit Fest app?
>
> If you don't see any other issue with the patch, I'd say yes. A
> committer can still put it back to waiting on author if needed.
>
That's also what I thought, but as I was the only one commenting on this...
Anyway, done.
--
Guillaume.
Re: Probable memory leak with ECPG and AIX
Le dim. 2 janv. 2022 à 01:07, Noah Misch a écrit : > On Sat, Jan 01, 2022 at 11:35:02AM -0500, Tom Lane wrote: > > Noah Misch writes: > > > I get the same results. The leak arises because AIX freelocale() > doesn't free > > > all memory allocated in newlocale(). The following program uses > trivial > > > memory on GNU/Linux, but it leaks like you're seeing on AIX: > > > > Bleah. > > > > > If you have access to file an AIX bug, I recommend doing so. If we > want > > > PostgreSQL to work around this, one idea is to have ECPG do this > newlocale() > > > less often. For example, do it once per process or once per connection > > > instead of once per ecpg_do_prologue(). > > > > It's worse than that: see also ECPGget_desc(). Seems like a case > > could be made for doing something about this just on the basis > > of cycles expended, never mind freelocale() bugs. > > Agreed. Once per process seems best. I only hesitated before since it > means > nothing will free this storage, which could be annoying in the context of > Valgrind and similar. However, ECPG already has bits of never-freed > memory in > the form of pthread_key_create() calls having no pthread_key_delete(), so I > don't mind adding a bit more. > Did this get anywhere? Is there something we could do to make this move forward? -- Guillaume.
Re: Probable memory leak with ECPG and AIX
Le ven. 25 mars 2022, 14:25, Tom Lane a écrit : > Guillaume Lelarge writes: > > Did this get anywhere? Is there something we could do to make this move > > forward? > > No. Write a patch? > I wouldn't have asked if I could write such a patch :-)
Re: REINDEX blocks virtually any queries but some prepared queries.
Le jeu. 7 avr. 2022 à 15:44, Frédéric Yhuel a écrit : > > > On 4/7/22 14:40, Justin Pryzby wrote: > > On Thu, Apr 07, 2022 at 01:37:57PM +0200, Frédéric Yhuel wrote: > >> Maybe something along this line? (patch attached) > > Some language fixes. > > Thank you Justin! I applied your fixes in the v2 patch (attached). > > v2 patch sounds good. > > I didn't verify the behavior, but +1 to document the practical > consequences. > > I guess this is why someone invented REINDEX CONCURRENTLY. > > > > Indeed ;) That being said, REINDEX CONCURRENTLY could give you an > invalid index, so sometimes you may be tempted to go for a simpler > REINDEX, especially if you believe that the SELECTs won't be blocked. Agreed. -- Guillaume.
pgbench translation
Hello, I've been working yesterday on the translation of v15 .po french files. Once again, I've found that there was no .po file for oid2name, pgbench, and vacuumlo. I suppose that's because they are contrib applications. But pgbench is no longer a contrib application. It's now located in the "src/bin/pgbench" directory. So I'm wondering if it should be translatable? If yes, I could work on that. Thanks. Regards. -- Guillaume.
Re: pgbench translation
Le mer. 13 avr. 2022 à 11:29, Alvaro Herrera a écrit : > On 2022-Apr-13, Guillaume Lelarge wrote: > > > Once again, I've found that there was no .po file for oid2name, pgbench, > > and vacuumlo. I suppose that's because they are contrib applications. But > > pgbench is no longer a contrib application. It's now located in the > > "src/bin/pgbench" directory. So I'm wondering if it should be > translatable? > > If yes, I could work on that. > > I think pgbench should be translatable, but last I looked at the sources, > there's *a lot* of work to put it in good shape for translatability. > There are many messages constructed from pieces, some terminology is > inconsistent, and other problems. If you want to work on fixing those > problems, +1 from me -- but let's get the problems fixed ahead of > adding it to the translation catalogs. > > Yeah, I didn't look closely at pgbench's source code. Just enough to make sure it wasn't written to be translated. On the messages needing some fixes, I don't know if I can do it. The terminology is quite foreign to me. Translating the man page has already been a nightmare :) Anyway, I'll get a look and see what I can do. Thanks for the answer. -- Guillaume.
Issue attaching a table to a partitioned table with an auto-referenced foreign key
Hello,
One of our customers has an issue with partitions and foreign keys. He
works on a v13, but the issue is also present on v15.
I attach a SQL script showing the issue, and the results on 13.7, 13.9, and
15.1. But I'll explain the script here, and its behaviour on 13.9.
There is one partitioned table, two partitions and a foreign key. The
foreign key references the same table:
create table t1 (
c1 bigint not null,
c1_old bigint null,
c2 bigint not null,
c2_old bigint null,
primary key (c1, c2)
)
partition by list (c1);
create table t1_a partition of t1 for values in (1);
create table t1_def partition of t1 default;
alter table t1 add foreign key (c1_old, c2_old) references t1 (c1, c2) on
delete restrict on update restrict;
I've a SQL function that shows me some information from pg_constraints
(code of the function in the SQL script attached). Here is the result of
this function after creating the table, its partitions, and its foreign key:
select * from show_constraints();
conname | t| tref | coparent
+++---
t1_c1_old_c2_old_fkey | t1 | t1 |
t1_c1_old_c2_old_fkey | t1_a | t1 | t1_c1_old_c2_old_fkey
t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
t1_c1_old_c2_old_fkey1 | t1 | t1_a | t1_c1_old_c2_old_fkey
t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
(5 rows)
The constraint works great :
insert into t1 values(1, NULL, 2, NULL);
insert into t1 values(2, 1,2, 2);
delete from t1 where c1 = 1;
psql:ticket15010_v3.sql:34: ERROR: update or delete on table "t1_a"
violates foreign key constraint "t1_c1_old_c2_old_fkey1" on table "t1"
DETAIL: Key (c1, c2)=(1, 2) is still referenced from table "t1".
This error is normal since the line I want to delete is referenced on the
other line.
If I try to detach the partition, it also gives me an error.
alter table t1 detach partition t1_a;
psql:ticket15010_v3.sql:36: ERROR: removing partition "t1_a" violates
foreign key constraint "t1_c1_old_c2_old_fkey1"
DETAIL: Key (c1_old, c2_old)=(1, 2) is still referenced from table "t1".
Sounds good to me too (well, I'd like it to be smarter and find that the
constraint is still good after the detach, but I can understand why it
won't allow it).
The pg_constraint didn't change of course:
select * from show_constraints();
conname | t| tref | coparent
+++---
t1_c1_old_c2_old_fkey | t1 | t1 |
t1_c1_old_c2_old_fkey | t1_a | t1 | t1_c1_old_c2_old_fkey
t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
t1_c1_old_c2_old_fkey1 | t1 | t1_a | t1_c1_old_c2_old_fkey
t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
(5 rows)
Now, I'll delete the whole table contents, and I'll detach the partition:
delete from t1;
alter table t1 detach partition t1_a;
It seems to be working, but the content of pg_constraints is weird:
select * from show_constraints();
conname | t| tref | coparent
+++---
t1_c1_old_c2_old_fkey | t1 | t1 |
t1_c1_old_c2_old_fkey | t1_a | t1 |
t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
(4 rows)
I understand why the ('t1_c1_old_c2_old_fkey1', 't1', 't1_a',
't1_c1_old_c2_old_fkey') tuple has gone but I don't understand why the
('t1_c1_old_c2_old_fkey', 't1_a', 't1', NULL) tuple is still there.
Anyway, I attach the partition:
alter table t1 attach partition t1_a for values in (1);
But pg_constraint has not changed:
select * from show_constraints();
conname | t| tref | coparent
+++---
t1_c1_old_c2_old_fkey | t1 | t1 |
t1_c1_old_c2_old_fkey | t1_a | t1 | t1_c1_old_c2_old_fkey
t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
(4 rows)
I was expecting to see the fifth tuple coming back, but alas, no.
And as a result, the foreign key doesn't work anymore:
insert into t1 values(1, NULL, 2, NULL);
insert into t1 values(2, 1,2, 2);
delete from t1 where c1 = 1;
Well, let's truncate the partitioned table, and drop the partition:
truncate t1;
drop table t1_a;
The content of pg_constraint looks good to me:
select * from show_constraints();
conname | t| tref | coparent
+++---
t1_c1_old_c2_old_fkey | t1 | t1 |
t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
(3 rows)
Let's create the partition to see if that works better:
create table t1_a
Re: Issue attaching a table to a partitioned table with an auto-referenced foreign key
Quick ping, just to make sure someone can get a look at this issue :)
Thanks.
Le ven. 6 janv. 2023 à 11:07, Guillaume Lelarge a
écrit :
> Hello,
>
> One of our customers has an issue with partitions and foreign keys. He
> works on a v13, but the issue is also present on v15.
>
> I attach a SQL script showing the issue, and the results on 13.7, 13.9,
> and 15.1. But I'll explain the script here, and its behaviour on 13.9.
>
> There is one partitioned table, two partitions and a foreign key. The
> foreign key references the same table:
>
> create table t1 (
> c1 bigint not null,
> c1_old bigint null,
> c2 bigint not null,
> c2_old bigint null,
> primary key (c1, c2)
> )
> partition by list (c1);
> create table t1_a partition of t1 for values in (1);
> create table t1_def partition of t1 default;
> alter table t1 add foreign key (c1_old, c2_old) references t1 (c1, c2) on
> delete restrict on update restrict;
>
> I've a SQL function that shows me some information from pg_constraints
> (code of the function in the SQL script attached). Here is the result of
> this function after creating the table, its partitions, and its foreign key:
>
> select * from show_constraints();
> conname | t| tref | coparent
> +++---
> t1_c1_old_c2_old_fkey | t1 | t1 |
> t1_c1_old_c2_old_fkey | t1_a | t1 | t1_c1_old_c2_old_fkey
> t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
> t1_c1_old_c2_old_fkey1 | t1 | t1_a | t1_c1_old_c2_old_fkey
> t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
> (5 rows)
>
> The constraint works great :
>
> insert into t1 values(1, NULL, 2, NULL);
> insert into t1 values(2, 1,2, 2);
> delete from t1 where c1 = 1;
> psql:ticket15010_v3.sql:34: ERROR: update or delete on table "t1_a"
> violates foreign key constraint "t1_c1_old_c2_old_fkey1" on table "t1"
> DETAIL: Key (c1, c2)=(1, 2) is still referenced from table "t1".
>
> This error is normal since the line I want to delete is referenced on the
> other line.
>
> If I try to detach the partition, it also gives me an error.
>
> alter table t1 detach partition t1_a;
> psql:ticket15010_v3.sql:36: ERROR: removing partition "t1_a" violates
> foreign key constraint "t1_c1_old_c2_old_fkey1"
> DETAIL: Key (c1_old, c2_old)=(1, 2) is still referenced from table "t1".
>
> Sounds good to me too (well, I'd like it to be smarter and find that the
> constraint is still good after the detach, but I can understand why it
> won't allow it).
>
> The pg_constraint didn't change of course:
>
> select * from show_constraints();
> conname | t| tref | coparent
> +++---
> t1_c1_old_c2_old_fkey | t1 | t1 |
> t1_c1_old_c2_old_fkey | t1_a | t1 | t1_c1_old_c2_old_fkey
> t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
> t1_c1_old_c2_old_fkey1 | t1 | t1_a | t1_c1_old_c2_old_fkey
> t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
> (5 rows)
>
> Now, I'll delete the whole table contents, and I'll detach the partition:
>
> delete from t1;
> alter table t1 detach partition t1_a;
>
> It seems to be working, but the content of pg_constraints is weird:
>
> select * from show_constraints();
> conname | t| tref | coparent
> +++---
> t1_c1_old_c2_old_fkey | t1 | t1 |
> t1_c1_old_c2_old_fkey | t1_a | t1 |
> t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
> t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
> (4 rows)
>
> I understand why the ('t1_c1_old_c2_old_fkey1', 't1', 't1_a',
> 't1_c1_old_c2_old_fkey') tuple has gone but I don't understand why the
> ('t1_c1_old_c2_old_fkey', 't1_a', 't1', NULL) tuple is still there.
>
> Anyway, I attach the partition:
>
> alter table t1 attach partition t1_a for values in (1);
>
> But pg_constraint has not changed:
>
> select * from show_constraints();
> conname | t| tref | coparent
> +++---
> t1_c1_old_c2_old_fkey | t1 | t1 |
> t1_c1_old_c2_old_fkey | t1_a | t1 | t1_c1_old_c2_old_fkey
> t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
> t1_c1_old_c2_old_fkey2 | t1 | t1_
Quick doc typo fix
Issue found while translating the v12 manual. I also fixed something that was missing, as far as I understand it (first fix, the typo is the second fix). See patch attached. Thanks. -- Guillaume. diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 4c7e93892a..f81f3d4160 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -57,7 +57,7 @@ pg_am - index access methods + table and index access methods diff --git a/doc/src/sgml/tablesample-method.sgml b/doc/src/sgml/tablesample-method.sgml index 880e42c950..68eea68077 100644 --- a/doc/src/sgml/tablesample-method.sgml +++ b/doc/src/sgml/tablesample-method.sgml @@ -264,7 +264,7 @@ NextSampleTuple (SampleScanState *node, requests to sample missing or invisible tuples; that should not result in any bias in the sample. However, if necessary, the function can use node->donetuples to examine how many of the tuples -it returned were vlaid and visible. +it returned were valid and visible.
Re: Quick doc typo fix
Le mar. 28 mai 2019 à 21:28, Michael Paquier a écrit : > On Tue, May 28, 2019 at 05:05:10PM +0200, Guillaume Lelarge wrote: > > > > linkend="catalog-pg-am">pg_am > > - index access methods > > + table and index access methods > > > > Perhaps we could just say "relation" here? That's the term used on > the paragraph describing pg_am. > Hehe, that was the first thing I wrote :) but went with "table and index" as it was also used a bit later in the chapter. Both are fine with me. -- Guillaume.
Re: Quick doc typo fix
Le mar. 28 mai 2019 à 21:46, Guillaume Lelarge a écrit : > Le mar. 28 mai 2019 à 21:28, Michael Paquier a > écrit : > >> On Tue, May 28, 2019 at 05:05:10PM +0200, Guillaume Lelarge wrote: >> > >> >> linkend="catalog-pg-am">pg_am >> > - index access methods >> > + table and index access methods >> > >> >> Perhaps we could just say "relation" here? That's the term used on >> the paragraph describing pg_am. >> > > Hehe, that was the first thing I wrote :) but went with "table and index" > as it was also used a bit later in the chapter. Both are fine with me. > > And here is another one. See patch attached. -- Guillaume. diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml index 2413089ffe..4c34ad9cc9 100644 --- a/doc/src/sgml/file-fdw.sgml +++ b/doc/src/sgml/file-fdw.sgml @@ -174,7 +174,7 @@ - COPY's FORCE_QUOTE options is + COPY's FORCE_QUOTE option is currently not supported by file_fdw.
Re: Quick doc typo fix
Le mer. 29 mai 2019 19:45, Michael Paquier a écrit : > On Wed, May 29, 2019 at 05:30:33PM +0200, Guillaume Lelarge wrote: > > And here is another one. See patch attached. > > Are you still going through some parts of the documentation? Perhaps > you may notice something else? I am wondering if it would be better > to wait a bit more so as we can group all issues you are finding at > once. > Yeah, I still have quite a lot to process. That might be better to do it all in once.
Re: Quick doc typo fix
Le mer. 29 mai 2019 19:52, Michael Paquier a écrit : > On Wed, May 29, 2019 at 07:47:12PM +0200, Guillaume Lelarge wrote: > > Yeah, I still have quite a lot to process. That might be better to do it > > all in once. > > OK, thanks! Could you ping me on this thread once you think you are > done? > Sure.
Re: Quick doc typo fix
Le sam. 8 juin 2019 à 15:44, Julien Rouhaud a écrit : > On Sat, Jun 8, 2019 at 3:33 PM Michael Paquier > wrote: > > > > Hi Guillaume, > > > > On Wed, May 29, 2019 at 08:28:59PM +0200, Guillaume Lelarge wrote: > > > Sure. > > > > I have noticed your message on the French list about the completion of > > the traduction, and congrats for that, it is a huge amount of work. > > Did you find anything else after your last report? > > I have two more fixes. See attached patch. It was the merge of upstream documentation completion in the french > repo, so the actual translation work can now begin. Probably there > will be more typo finding in the next weeks. > Yeah, only the merge is done. We now need to work on the actual translation. But this is way more fun than the merge :) We might find more typos, but it will take time. Applying this patch now (if it fits you) is probably better. -- Guillaume. diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml index 2413089ffe..4c34ad9cc9 100644 --- a/doc/src/sgml/file-fdw.sgml +++ b/doc/src/sgml/file-fdw.sgml @@ -174,7 +174,7 @@ - COPY's FORCE_QUOTE options is + COPY's FORCE_QUOTE option is currently not supported by file_fdw. diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index e053e2ee34..323621b95c 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -753,7 +753,7 @@ psql: could not connect to server: No such file or directory SEMMNI Maximum number of semaphore identifiers (i.e., sets) -at least ceil((max_connections + autovacuum_max_workers + wax_wal_senders + max_worker_processes + 5) / 16) plus room for other applications +at least ceil((max_connections + autovacuum_max_workers + max_wal_senders + max_worker_processes + 5) / 16) plus room for other applications
Re: Quick doc typo fix
Le dim. 9 juin 2019 à 04:27, Michael Paquier a écrit : > On Sat, Jun 08, 2019 at 04:23:55PM +0200, Guillaume Lelarge wrote: > > We might find more typos, but it will take time. Applying this patch now > > (if it fits you) is probably better. > > I can imagine that it is a daunting task... Ok, for now I have > applied what you sent. Thanks! > Thank you. -- Guillaume.
Re: Quitting the thes
Le mer. 12 juin 2019 à 20:45, Alvaro Herrera a écrit : > On 2019-May-14, Michael Paquier wrote: > > > Thanks, committed. I have noticed an extra one in reorderbuffer.c. > > Some grepping found a bit more; patch attached. > > Thanks a lot, this is very good. I've got some fixes to do :) -- Guillaume.
Re: min_wal_size > max_wal_size is accepted
Hi, Le jeu. 7 mai 2020 à 11:13, Marc Rechté a écrit : > Hello, > > It is possible to startup an instance with min > max, without the system > complaining: > > mrechte=# show min_wal_size ; > > 2020-05-07 11:12:11.422 CEST [11098] LOG: durée : 0.279 ms > > min_wal_size > > -- > > 128MB > > (1 ligne) > > > > mrechte=# show max_wal_size ; > > 2020-05-07 11:12:12.814 CEST [11098] LOG: durée : 0.275 ms > > max_wal_size > > -- > > 64MB > > (1 ligne) > > > This could be an issue ? > > I don't see how this could be an issue. You'll get a checkpoint every time 64MB have been written before checkpoint_timeout kicked in. And WAL files will be removed if you have more than 128MB of them. Not the smartest configuration, but not a damaging one either. -- Guillaume.
Extensions not dumped when --schema is used
Hello,
I've discovered something today that I didn't really expect. When a user
dumps a database with the --schema flag of pg_dump, extensions in this
schema aren't dumped. As far as I can tell, the documentation isn't clear
about this ("Dump only schemas matching pattern; this selects both the
schema itself, and all its contained objects."), though the source code
definitely is ("We dump all user-added extensions by default, or none of
them if include_everything is false (i.e., a --schema or --table switch was
given).", in pg_dump.c).
I was wondering the reason behind this choice. If anyone knows, I'd be
happy to hear about it.
I see two things:
* it's been overlooked, and we should dump all the extensions available in
a schema if this schema has been selected through the --schema flag.
* it's kind of like the large objects handling, and I'd pretty interested
in adding a --extensions (as the same way there is a --blobs flag).
Thanks.
Regards.
--
Guillaume.
Re: Extensions not dumped when --schema is used
Le mer. 20 mai 2020 à 11:26, Daniel Gustafsson a écrit : > > On 20 May 2020, at 10:06, Guillaume Lelarge > wrote: > > > I was wondering the reason behind this choice. If anyone knows, I'd be > happy to hear about it. > > Extensions were dumped unconditionally in the beginning, but it was > changed to > match how procedural language definitions were dumped. > > That makes sense. Thank you. > * it's been overlooked, and we should dump all the extensions available > in a schema if this schema has been selected through the --schema flag. > > For reference, --schema-only will include all the extensions, but not > --schema=foo and not "--schema-only --schema=foo". > > Yes. Extensions don't belong to a schema per se, the namespace oid in > pg_extension > marks where most of the objects are contained but not necessarily all of > them. > Given that, it makes sense to not include extensions for --schema. > However, > that can be considered sort of an implementation detail which may not be > entirely obvious to users (especially since you yourself is a power-user). > > I agree. > * it's kind of like the large objects handling, and I'd pretty interested > in adding a --extensions (as the same way there is a --blobs flag). > > An object in a schema might have attributes that depend on an extension in > order to restore, so it makes sense to provide a way to include them for a > --schema dump. That's actually how I figured this out. A customer can't restore his dump because of a missing extension (pg_trgm to be precise). The question is what --extensions should do: only dump any > extensions that objects in the schema depend on; require a pattern and only > dump matching extensions; dump all extensions (probably not) or something > else? > > Actually, "dump all extensions" (#3) would make sense to me, and has my vote. Otherwise, and though it would imply much more work, "only dump any extension that objects in the schema depend on" (#1) comes second in my opinion. Using the pattern means something more manual for users, and I really think it would be a bad idea. People dump databases, schemas, and tables. Theu usually don't know which extensions those objects depend on. But, anyway, I would work on any of these solutions, depending on what most people think is best. Thanks. -- Guillaume.
Re: Extensions not dumped when --schema is used
Le mer. 20 mai 2020 à 16:39, Tom Lane a écrit : > Guillaume Lelarge writes: > > Le mer. 20 mai 2020 à 11:26, Daniel Gustafsson a > écrit : > >> The question is what --extensions should do: only dump any > >> extensions that objects in the schema depend on; require a pattern and > only > >> dump matching extensions; dump all extensions (probably not) or > something > >> else? > > > Actually, "dump all extensions" (#3) would make sense to me, and has my > > vote. > > I think that makes no sense at all. By definition, a dump that's been > restricted with --schema, --table, or any similar switch is incomplete > and may not restore on its own. Typical examples include foreign key > references to tables in other schemas, views using functions in other > schemas, etc etc. I see no reason for extension dependencies to be > treated differently from those cases. > > Agreed. In any use of selective dump, it's the user's job to select a set of > objects that she wants dumped (or restored). Trying to second-guess that > is mostly going to make the feature less usable for power-user cases. > > Agreed, though right now he has no way to do this for extensions. As a counterexample, what if you want the dump to be restorable on a > system that doesn't have all of the extensions available on the source? > You carefully pick out the tables that you need, which don't require the > unavailable extensions ... and then pg_dump decides you don't know what > you're doing and includes all the problematic extensions anyway. > > That's true. I could get behind an "--extensions=PATTERN" switch to allow selective > addition of extensions to a selective dump, but I don't want to see us > overruling the user's choices about what to dump. > > With all your comments, I can only agree to your views. I'll try to work on this anytime soon. -- Guillaume.
Re: Default gucs for EXPLAIN
Le mar. 26 mai 2020 à 04:27, Stephen Frost a écrit : > Greetings, > > * Michael Paquier (mich...@paquier.xyz) wrote: > > On Mon, May 25, 2020 at 09:36:50PM -0400, Bruce Momjian wrote: > > > I am not excited about this new feature. Why do it only for EXPLAIN? > > Would probably help to understand what your thinking is here regarding > how it could be done for everything...? In particular, what else are > you thinking it'd be sensible for? > > > > That is a log of GUCs. I can see this becoming a feature creep > > > disaster. > > I'd only view it as a feature creep disaster if we end up extending it > to things that don't make any sense.. I don't see any particular reason > why we'd have to do that though. On the other hand, if there's a clean > way to do it for everything, that'd be pretty neat. > > > FWIW, Neither am I. This would create an extra maintenance cost, and > > I would not want such stuff to spread to other commands either, say > > CLUSTER, VACUUM, REINDEX, etc. And note that it is always possible to > > do that with a small extension using the utility hook and some > > pre-loaded user-settable GUCs. > > The suggestion to "go write C code that will be loaded via a utility > hook" is really entirely inappropriate here. > > This strikes me as a pretty reasonable 'creature comfort' kind of idea. > Inventing GUCs to handle it is maybe not the best approach, but we > haven't really got anything better right at hand- psql can't parse > general SQL, today, and hasn't got it's own idea of "how to run > explain". On the other hand, I could easily see a similar feature > being included in pgAdmin4 where running explain is clicking on a button > instead of typing 'explain'. > > To that end- what if this was done client-side with '\explain' or > similar? Basically, it'd work like \watch or \g but we'd have options > under pset like "explain_analyze t/f" and such. I feel like that'd also > largely address the concerns about how this might 'feature creep' to > other commands- because those other commands don't work with a query > buffer, so it wouldn't really make sense for them. > > As for the concerns wrt explain UPDATE or explain DETELE actually > running the query, that's what transactions are for, and if you don't > feel comfortable using transactions or using these options- then don't. > > This means you'll always have to check if the new GUCs are set up in a way it will actually execute the query or to open a transaction for the same reason. This is a huge behaviour change where people might lose data. I really don't like this proposal (the new GUCs). -- Guillaume.
Re: Default gucs for EXPLAIN
Le mar. 26 mai 2020 à 16:25, Stephen Frost a écrit : > Greetings, > > * Guillaume Lelarge (guilla...@lelarge.info) wrote: > > Le mar. 26 mai 2020 à 04:27, Stephen Frost a écrit > : > > > To that end- what if this was done client-side with '\explain' or > > > similar? Basically, it'd work like \watch or \g but we'd have options > > > under pset like "explain_analyze t/f" and such. I feel like that'd > also > > > largely address the concerns about how this might 'feature creep' to > > > other commands- because those other commands don't work with a query > > > buffer, so it wouldn't really make sense for them. > > > > > > As for the concerns wrt explain UPDATE or explain DETELE actually > > > running the query, that's what transactions are for, and if you don't > > > feel comfortable using transactions or using these options- then don't. > > > > This means you'll always have to check if the new GUCs are set up in a > way > > it will actually execute the query or to open a transaction for the same > > reason. This is a huge behaviour change where people might lose data. > > It's only a behaviour change if you enable it.. and the suggestion I > made specifically wouldn't even be a regular 'explain', you'd be using > '\explain' in psql, a new command. > > > I really don't like this proposal (the new GUCs). > > The proposal you're commenting on (seemingly mine, anyway) didn't > include adding any new GUCs. > > My bad. I didn't read your email properly, sorry. I wouldn't complain about a \explain metacommand. The proposal I (still) dislike is Vik's. -- Guillaume.
Re: Default gucs for EXPLAIN
Le mer. 3 juin 2020 à 04:16, David Fetter a écrit : > On Tue, Jun 02, 2020 at 09:28:48PM +0200, Vik Fearing wrote: > > On 6/2/20 7:54 PM, David G. Johnston wrote: > > > At this point, given the original goal of the patch was to try and > > > grease a smoother path to changing the default for BUFFERS, and > > > that people seem OK with doing just that without having this > > > patch, I'd say we should just change the default and forget this > > > patch. There hasn't been any other demand from our users for this > > > capability and I also doubt that having BUFFERS on by default is > > > going to bother people. > > > > What about WAL? Can we turn that one one by default, too? > > > > I often find having VERBOSE on helps when people don't qualify their > > columns and I don't know the schema. We should turn that on by > > default, as well. > > +1 for all on (except ANALYZE because it would be a foot cannon) by > default. For those few to whom it really matters, there'd be OFF > switches. > > +1 for all on, except ANALYZE (foot cannon as David says) and VERBOSE (verbose is something you ask for when the usual display isn't enough). -1 for GUCs, we already have too many of them. -- Guillaume.
Re: parallel vacuum options/syntax
Le jeu. 2 janv. 2020 à 13:09, Amit Kapila a écrit : > Hi, > > I am starting a new thread for some of the decisions for a parallel vacuum > in the hope to get feedback from more people. There are mainly two points > for which we need some feedback. > > 1. Tomas Vondra has pointed out on the main thread [1] that by default the > parallel vacuum should be enabled similar to what we do for Create Index. > As proposed, the patch enables it only when the user specifies it (ex. > Vacuum (Parallel 2) ;). One of the arguments in favor of > enabling it by default as mentioned by Tomas is "It's pretty much the same > thing we did with vacuum throttling - it's disabled for explicit vacuum by > default, but you can enable it. If you're worried about VACUUM causing > issues, you should set cost delay.". Some of the arguments against > enabling it are that it will lead to use of more resources (like CPU, I/O) > which users might or might like. > > Now, if we want to enable it by default, we need a way to disable it as > well and along with that, we need a way for users to specify a parallel > degree. I have mentioned a few reasons why we need a parallel degree for > this operation in the email [2] on the main thread. > > If parallel vacuum is **not** enabled by default, then I think the > current way to enable is fine which is as follows: > Vacuum (Parallel 2) ; > > Here, if the user doesn't specify parallel_degree, then we internally > decide based on number of indexes that support a parallel vacuum with a > maximum of max_parallel_maintenance_workers. > > If the parallel vacuum is enabled by default, then I could think of the > following ways: > (a) Vacuum (disable_parallel) ; Vacuum (Parallel > ) ; > (b) Vacuum (Parallel ) ; If user specifies > parallel_degree as 0, then disable parallelism. > (c) ... Any better ideas? > > AFAICT, every parallel-able statement use parallelisation by default, so it wouldn't be consistent if VACUUM behaves some other way. So, (c) has my vote. 2. The patch provides a FAST option (based on suggestion by Robert) for a > parallel vacuum which will make it behave like vacuum_cost_delay = 0 which > means it will disable throttling. So, > VACUUM (PARALLEL n, FAST) will allow the parallel vacuum to run > without resource throttling. Tomas thinks that we don't need such an > option as the same can be served by setting vacuum_cost_delay = 0 which is > a valid argument, but OTOH, providing an option to the user which can make > his life easier is not a bad idea either. > > The user already has an option (the vacuum_cost_delay GUC). So I kinda agree with Tomas on this. -- Guillaume.
Re: parallel vacuum options/syntax
Le ven. 3 janv. 2020 à 09:06, Amit Kapila a écrit : > On Thu, Jan 2, 2020 at 7:09 PM Guillaume Lelarge > wrote: > > > > Le jeu. 2 janv. 2020 à 13:09, Amit Kapila a > écrit : > >> > >> If parallel vacuum is *not* enabled by default, then I think the > current way to enable is fine which is as follows: > >> Vacuum (Parallel 2) ; > >> > >> Here, if the user doesn't specify parallel_degree, then we internally > decide based on number of indexes that support a parallel vacuum with a > maximum of max_parallel_maintenance_workers. > >> > >> If the parallel vacuum is enabled by default, then I could think of the > following ways: > >> (a) Vacuum (disable_parallel) ; Vacuum (Parallel > ) ; > >> (b) Vacuum (Parallel ) ; If user specifies > parallel_degree as 0, then disable parallelism. > >> (c) ... Any better ideas? > >> > > > > AFAICT, every parallel-able statement use parallelisation by default, so > it wouldn't be consistent if VACUUM behaves some other way. > > > > Fair enough. > > > So, (c) has my vote. > > > > I don't understand this. What do you mean by voting (c) option? Do > you mean that you didn't like any of (a) or (b)? I meant (b), sorry :) If so, then feel > free to suggest something else. One more possibility could be to > allow users to specify parallel degree or disable parallelism via guc > 'max_parallel_maintenance_workers'. Basically, if the user wants to > disable parallelism, it needs to set the value of guc > max_parallel_maintenance_workers as zero and if it wants to increase > the parallel degree than the default value (which is two), then it can > set it via max_parallel_maintenance_workers before running vacuum > command. Now, this can certainly work, but I feel setting/resetting a > guc before a vacuum > command can be a bit inconvenient for users, but if others prefer that > way, then we can do that. > > -- Guillaume.
Quick doc patch
Hi, Here is a quick issue I found on the BRIN documentation. I'm not a 100% sure I'm right but it looks like a failed copy/paste from the GIN documentation. Cheers. -- Guillaume. diff --git a/doc/src/sgml/brin.sgml b/doc/src/sgml/brin.sgml index 4c5eeb875f..55b6272db6 100644 --- a/doc/src/sgml/brin.sgml +++ b/doc/src/sgml/brin.sgml @@ -585,7 +585,7 @@ typedef struct BrinOpcInfo Since both key extraction of indexed values and representation of the - key in GIN are flexible, they may depend on + key in BRIN are flexible, they may depend on user-specified parameters.
Weird use of parentheses in the manual
Hey, I was translating logicaldecoding.sgml, and I saw this sentence: There are multiple required streaming callbacks (stream_start_cb, stream_stop_cb, stream_abort_cb, stream_commit_cb and stream_change_cb) and two optional callbacks (stream_message_cb) and (stream_truncate_cb). The two last sets of parentheses seem really weird to me. Looks like it should be: (stream_message_cb and stream_truncate_cb). Really tiny patch attached to fix this if it really is wrong, and anyone cares enough to fix it :) Regards. -- Guillaume. diff --git a/doc/src/sgml/logicaldecoding.sgml b/doc/src/sgml/logicaldecoding.sgml index 1765ea6c87..f2d3f1c6a8 100644 --- a/doc/src/sgml/logicaldecoding.sgml +++ b/doc/src/sgml/logicaldecoding.sgml @@ -1173,7 +1173,7 @@ OutputPluginWrite(ctx, true); (stream_start_cb, stream_stop_cb, stream_abort_cb, stream_commit_cb and stream_change_cb) and two optional callbacks -(stream_message_cb) and (stream_truncate_cb). +(stream_message_cb and stream_truncate_cb).
Re: Weird use of parentheses in the manual
Le ven. 25 juin 2021 à 08:55, Amit Kapila a écrit : > On Thu, Jun 24, 2021 at 6:38 PM Amit Kapila > wrote: > > > > On Thu, Jun 24, 2021 at 5:27 PM Guillaume Lelarge > > wrote: > > > > > > Really tiny patch attached to fix this if it really is wrong, and > anyone cares enough to fix it :) > > > > > > > LGTM. I'll take care of this tomorrow unless someone else has any > > suggestions in this regard. > > > > Pushed. > > Thanks. -- Guillaume.
Re: New committers: Daniel Gustafsson and John Naylor
Le mer. 30 juin 2021 à 22:44, Tom Lane a écrit : > The Core Team would like to extend our congratulations to > Daniel Gustafsson and John Naylor, who have accepted invitations > to become our newest Postgres committers. > > Please join me in wishing them much success and few bugs. > > Congrats to both. I guess https://wiki.postgresql.org/wiki/Committers needs to be updated ;) -- Guillaume.
Re: Proposal: adding a better description in psql command about large objects
Le ven. 3 juin 2022 à 19:29, Nathan Bossart a écrit : > On Fri, Jun 03, 2022 at 12:56:20PM -0400, Tom Lane wrote: > > Nathan Bossart writes: > >> Another option could be to move it after the "Input/Output" section so > that > >> it's closer to some other commands that involve files. I can't say I > have > >> a strong opinion about whether/where to move it, though. > > > > Yeah, I thought of that choice too, but it ends up placing the > > Large Objects section higher up the list than seems warranted on > > frequency-of-use grounds. > > Fair point. > > > After looking at the output I concluded that we'd be better off to > > stick with the normal indentation amount, and break the lo_import > > entry into two lines to make that work. One reason for this is > > that some translators might've already settled on a different > > indentation amount in order to cope with translated parameter names, > > and deviating from the normal here will just complicate their lives. > > So that leaves me proposing v5. > > I see. As you noted earlier, moving the entries higher makes the > inconsistent indentation less appealing, too. So this LGTM. > > Sounds good to me too. Thanks. -- Guillaume.
Add parallel columns for seq scan and index scan on pg_stat_all_tables and _indexes
time I had to add new columns to a statistics catalog. I'm actually not sure that we were right to change pg_proc.dat manually. We'll probably have to fix this. Documentation is done, but maybe we should also add that seq_scan and idx_scan also include parallel scan. Yet to be done: tests. Once there's an agreement on this patch, we'll work on the tests. This has been a collective work with Benoit Lobréau, Jehan-Guillaume de Rorthais, and Franck Boudehen. Thanks. Regards. [1] https://www.postgresql.org/message-id/flat/b4220d15-2e21-0e98-921b-b9892543cc93%40dalibo.com [2] https://www.postgresql.org/message-id/flat/d657df20-c4bf-63f6-e74c-cb85a81d0383%40dalibo.com -- Guillaume. test.sql Description: application/sql From 78fb5406c42c9ecd429e08314d9f3a0bfd112c6a Mon Sep 17 00:00:00 2001 From: Guillaume Lelarge Date: Wed, 28 Aug 2024 21:35:30 +0200 Subject: [PATCH] Add parallel columns for pg_stat_all_tables,indexes pg_stat_all_tables gets 4 new columns: parallel_seq_scan, last_parallel_seq_scan, parallel_idx_scan, last_parallel_idx_scan. pg_stat_all_indexes gets 2 new columns: parallel_idx_scan, last_parallel_idx_scan. --- doc/src/sgml/monitoring.sgml | 57 src/backend/access/heap/heapam.c | 2 + src/backend/access/nbtree/nbtsearch.c| 2 + src/backend/catalog/system_views.sql | 6 +++ src/backend/utils/activity/pgstat_relation.c | 7 ++- src/backend/utils/adt/pgstatfuncs.c | 6 +++ src/include/catalog/pg_proc.dat | 8 +++ src/include/pgstat.h | 13 + 8 files changed, 99 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 55417a6fa9..afd5a23528 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -3714,6 +3714,25 @@ description | Waiting for a newly initialized WAL file to reach durable storage + + + parallel_seq_scan bigint + + + Number of parallel sequential scans initiated on this table + + + + + + last_parallel_seq_scan timestamp with time zone + + + The time of the last parallel sequential scan on this table, based on the + most recent transaction stop time + + + seq_tup_read bigint @@ -3742,6 +3761,25 @@ description | Waiting for a newly initialized WAL file to reach durable storage + + + parallel_idx_scan bigint + + + Number of parallel index scans initiated on this table + + + + + + last_parallel_idx_scan timestamp with time zone + + + The time of the last parallel index scan on this table, based on the + most recent transaction stop time + + + idx_tup_fetch bigint @@ -4021,6 +4059,25 @@ description | Waiting for a newly initialized WAL file to reach durable storage + + + parallel_idx_scan bigint + + + Number of parallel index scans initiated on this index + + + + + + last_parallel_idx_scan timestamp with time zone + + + The time of the last parallel scan on this index, based on the + most recent transaction stop time + + + idx_tup_read bigint diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 91b20147a0..c2f1b8e25e 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -410,6 +410,8 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock) */ if (scan->rs_base.rs_flags & SO_TYPE_SEQSCAN) pgstat_count_heap_scan(scan->rs_base.rs_rd); + if (scan->rs_base.rs_parallel != NULL) + pgstat_count_parallel_heap_scan(scan->rs_base.rs_rd); } /* diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c index 2551df8a67..e37ed32bb1 100644 --- a/src/backend/access/nbtree/nbtsearch.c +++ b/src/backend/access/nbtree/nbtsearch.c @@ -897,6 +897,8 @@ _bt_first(IndexScanDesc scan, ScanDirection dir) Assert(!BTScanPosIsValid(so->currPos)); pgstat_count_index_scan(rel); + if (scan->parallel_scan != NULL) + pgstat_count_parallel_index_scan(rel); /* * Examine the scan keys and eliminate any redundant keys; also mark the diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 19cabc9a47..d78a121114 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -670,9 +670,13 @@ CREATE VIEW pg_stat_all_tables AS C.relname AS relname, pg_stat_get_numscans(C.oid) AS seq_scan, pg_stat_get_lastscan(C.oid) AS last_seq_scan, +pg_stat_get_parallelnumscans(C.oid) AS parallel_seq_scan, +pg_stat_get_parallellas
Add parallel columns for pg_stat_statements
ized_nodes_no_worker | 0 ==> another parallelized query ==> with 5 as max_parallel_workers_per_gather, but only 4 workers to use ==> hence four workers, with one node with all workers The biggest issue with this patch is that it's unable to know workers for maintenance queries (CREATE INDEX for BTree and VACUUM). Documentation is done, tests are missing. Once there's an agreement on this patch, we'll work on the tests. This has been a collective work with Benoit Lobréau, Jehan-Guillaume de Rorthais, and Franck Boudehen. Thanks. Regards. [1] https://www.postgresql.org/message-id/flat/b4220d15-2e21-0e98-921b-b9892543cc93%40dalibo.com [2] https://www.postgresql.org/message-id/flat/d657df20-c4bf-63f6-e74c-cb85a81d0383%40dalibo.com [3] https://www.postgresql.org/message-id/flat/6acbe570-068e-bd8e-95d5-00c737b865e8%40gmail.com -- Guillaume. From 59fd586cac8f0bafb1fa66548424f2dab0b38f31 Mon Sep 17 00:00:00 2001 From: Guillaume Lelarge Date: Wed, 28 Aug 2024 15:30:05 +0200 Subject: [PATCH] Add parallel columns to pg_stat_statements There are seven new columns: * parallelized_queries_planned (number of times the query has been planned to be parallelized), * parallel_ized_querieslaunched (number of times the query has been executed with parallelization), * parallelized_workers_planned (number of parallel workers planned for this query), * parallelized_workers_launched (number of parallel workers executed for this query), * parallelized_nodes (number of parallelized nodes), * parallelized_nodes_all_workers (number of parallelized nodes which had all requested workers), * parallelized_nodes_no_worker (number of parallelized nodes which had no requested workers). These new columns will help to monitor and better configure query parallelization. --- contrib/pg_stat_statements/Makefile | 2 +- .../pg_stat_statements--1.11--1.12.sql| 80 + .../pg_stat_statements/pg_stat_statements.c | 108 -- .../pg_stat_statements.control| 2 +- doc/src/sgml/pgstatstatements.sgml| 63 ++ src/backend/executor/execUtils.c | 7 ++ src/backend/executor/nodeGather.c | 9 +- src/backend/executor/nodeGatherMerge.c| 8 ++ src/include/nodes/execnodes.h | 6 + 9 files changed, 275 insertions(+), 10 deletions(-) create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.11--1.12.sql diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile index c19ccad77e..62f8df65b5 100644 --- a/contrib/pg_stat_statements/Makefile +++ b/contrib/pg_stat_statements/Makefile @@ -7,7 +7,7 @@ OBJS = \ EXTENSION = pg_stat_statements DATA = pg_stat_statements--1.4.sql \ - pg_stat_statements--1.10--1.11.sql \ + pg_stat_statements--1.11--1.12.sql pg_stat_statements--1.10--1.11.sql \ pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \ pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.sql \ pg_stat_statements--1.5--1.6.sql pg_stat_statements--1.4--1.5.sql \ diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.11--1.12.sql b/contrib/pg_stat_statements/pg_stat_statements--1.11--1.12.sql new file mode 100644 index 00..6f4fe8be48 --- /dev/null +++ b/contrib/pg_stat_statements/pg_stat_statements--1.11--1.12.sql @@ -0,0 +1,80 @@ +/* contrib/pg_stat_statements/pg_stat_statements--1.11--1.12.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION pg_stat_statements UPDATE TO '1.12'" to load this file. \quit + +/* First we have to remove them from the extension */ +ALTER EXTENSION pg_stat_statements DROP VIEW pg_stat_statements; +ALTER EXTENSION pg_stat_statements DROP FUNCTION pg_stat_statements(boolean); + +/* Then we can drop them */ +DROP VIEW pg_stat_statements; +DROP FUNCTION pg_stat_statements(boolean); + +/* Now redefine */ +CREATE FUNCTION pg_stat_statements(IN showtext boolean, +OUT userid oid, +OUT dbid oid, +OUT toplevel bool, +OUT queryid bigint, +OUT query text, +OUT plans int8, +OUT total_plan_time float8, +OUT min_plan_time float8, +OUT max_plan_time float8, +OUT mean_plan_time float8, +OUT stddev_plan_time float8, +OUT calls int8, +OUT total_exec_time float8, +OUT min_exec_time float8, +OUT max_exec_time float8, +OUT mean_exec_time float8, +OUT stddev_exec_time float8, +OUT rows int8, +OUT shared_blks_hit int8, +OUT shared_blks_read int8, +OUT shared_blks_dirtied int8, +OUT shared_blks_written int8, +OUT local_blks_hit int8, +OUT local_blks_read int8, +OUT local_blks_dirtied int8, +OUT local_blks_written int8, +OUT temp_blks_read int8, +OUT temp_blks_written int8, +OUT shared_blk_read_time float8, +OUT shared_blk_write_time float8, +OUT local_blk_read_time float8
Re: Issue attaching a table to a partitioned table with an auto-referenced foreign key
One last ping, hoping someone will have more time now than in january.
Perhaps my test is wrong, but I'd like to know why.
Thanks.
Le mar. 17 janv. 2023 à 16:53, Guillaume Lelarge a
écrit :
> Quick ping, just to make sure someone can get a look at this issue :)
> Thanks.
>
>
> Le ven. 6 janv. 2023 à 11:07, Guillaume Lelarge
> a écrit :
>
>> Hello,
>>
>> One of our customers has an issue with partitions and foreign keys. He
>> works on a v13, but the issue is also present on v15.
>>
>> I attach a SQL script showing the issue, and the results on 13.7, 13.9,
>> and 15.1. But I'll explain the script here, and its behaviour on 13.9.
>>
>> There is one partitioned table, two partitions and a foreign key. The
>> foreign key references the same table:
>>
>> create table t1 (
>> c1 bigint not null,
>> c1_old bigint null,
>> c2 bigint not null,
>> c2_old bigint null,
>> primary key (c1, c2)
>> )
>> partition by list (c1);
>> create table t1_a partition of t1 for values in (1);
>> create table t1_def partition of t1 default;
>> alter table t1 add foreign key (c1_old, c2_old) references t1 (c1, c2) on
>> delete restrict on update restrict;
>>
>> I've a SQL function that shows me some information from pg_constraints
>> (code of the function in the SQL script attached). Here is the result of
>> this function after creating the table, its partitions, and its foreign key:
>>
>> select * from show_constraints();
>> conname | t| tref | coparent
>> +++---
>> t1_c1_old_c2_old_fkey | t1 | t1 |
>> t1_c1_old_c2_old_fkey | t1_a | t1 | t1_c1_old_c2_old_fkey
>> t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
>> t1_c1_old_c2_old_fkey1 | t1 | t1_a | t1_c1_old_c2_old_fkey
>> t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
>> (5 rows)
>>
>> The constraint works great :
>>
>> insert into t1 values(1, NULL, 2, NULL);
>> insert into t1 values(2, 1,2, 2);
>> delete from t1 where c1 = 1;
>> psql:ticket15010_v3.sql:34: ERROR: update or delete on table "t1_a"
>> violates foreign key constraint "t1_c1_old_c2_old_fkey1" on table "t1"
>> DETAIL: Key (c1, c2)=(1, 2) is still referenced from table "t1".
>>
>> This error is normal since the line I want to delete is referenced on the
>> other line.
>>
>> If I try to detach the partition, it also gives me an error.
>>
>> alter table t1 detach partition t1_a;
>> psql:ticket15010_v3.sql:36: ERROR: removing partition "t1_a" violates
>> foreign key constraint "t1_c1_old_c2_old_fkey1"
>> DETAIL: Key (c1_old, c2_old)=(1, 2) is still referenced from table "t1".
>>
>> Sounds good to me too (well, I'd like it to be smarter and find that the
>> constraint is still good after the detach, but I can understand why it
>> won't allow it).
>>
>> The pg_constraint didn't change of course:
>>
>> select * from show_constraints();
>> conname | t| tref | coparent
>> +++---
>> t1_c1_old_c2_old_fkey | t1 | t1 |
>> t1_c1_old_c2_old_fkey | t1_a | t1 | t1_c1_old_c2_old_fkey
>> t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
>> t1_c1_old_c2_old_fkey1 | t1 | t1_a | t1_c1_old_c2_old_fkey
>> t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
>> (5 rows)
>>
>> Now, I'll delete the whole table contents, and I'll detach the partition:
>>
>> delete from t1;
>> alter table t1 detach partition t1_a;
>>
>> It seems to be working, but the content of pg_constraints is weird:
>>
>> select * from show_constraints();
>> conname | t| tref | coparent
>> +++---
>> t1_c1_old_c2_old_fkey | t1 | t1 |
>> t1_c1_old_c2_old_fkey | t1_a | t1 |
>> t1_c1_old_c2_old_fkey | t1_def | t1 | t1_c1_old_c2_old_fkey
>> t1_c1_old_c2_old_fkey2 | t1 | t1_def | t1_c1_old_c2_old_fkey
>> (4 rows)
>>
>> I understand why the ('t1_c1_old_c2_old_fkey1', 't1', 't1_a',
>> 't1_c1_old_c2_old_fkey') tuple has gone but I don't understand why the
>> ('t1_c1_old_c2_old_fkey', 't1_a', 't1', NU
Re: Add parallel columns for seq scan and index scan on pg_stat_all_tables and _indexes
Le mer. 4 sept. 2024 à 14:58, Bertrand Drouvot a écrit : > Hi, > > On Wed, Sep 04, 2024 at 02:51:51PM +0200, Guillaume Lelarge wrote: > > Le mer. 4 sept. 2024 ą 10:47, Bertrand Drouvot < > bertranddrouvot...@gmail.com> > > a écrit : > > > > > I don't see a CF entry for this patch. Would you mind creating one so > that > > > we don't lost track of it? > > > > > > > > I don't mind adding it, though I don't know if I should add it to the > > September or November commit fest. Which one should I choose? > > Thanks! That should be the November one (as the September one already > started). > > I should have gone to the commit fest website, it says the same. I had the recollection that it started on the 15th. Anyway, added to the november commit fest (https://commitfest.postgresql.org/50/5238/). -- Guillaume.
Re: Add parallel columns for seq scan and index scan on pg_stat_all_tables and _indexes
Hi,
Le mer. 4 sept. 2024 à 16:18, Bertrand Drouvot
a écrit :
> Hi,
>
> On Wed, Sep 04, 2024 at 02:51:51PM +0200, Guillaume Lelarge wrote:
> > Hi,
> >
> > Le mer. 4 sept. 2024 à 10:47, Bertrand Drouvot <
> bertranddrouvot...@gmail.com>
> > a écrit :
> >
> > > What about to get rid of the pgstat_count_parallel_heap_scan and add an
> > > extra
> > > bolean parameter to pgstat_count_heap_scan to indicate if
> > > counts.parallelnumscans
> > > should be incremented too?
> > >
> > > Something like:
> > >
> > > pgstat_count_heap_scan(scan->rs_base.rs_rd, scan->rs_base.rs_parallel
> !=
> > > NULL)
> > >
> > > 3 ===
> > >
> > > Same comment for pgstat_count_index_scan (add an extra bolean
> parameter)
> > > and
> > > get rid of pgstat_count_parallel_index_scan()).
> > >
> > > I think that 2 === and 3 === would help to avoid missing increments
> should
> > > we
> > > add those call to other places in the future.
> > >
> > >
> > Oh OK, understood. Done for both.
>
> Thanks for v2!
>
> 1 ===
>
> -#define pgstat_count_heap_scan(rel)
> +#define pgstat_count_heap_scan(rel, parallel)
> do {
> - if (pgstat_should_count_relation(rel))
> - (rel)->pgstat_info->counts.numscans++;
> +if (pgstat_should_count_relation(rel)) {
> + if (!parallel)
> + (rel)->pgstat_info->counts.numscans++;
> + else
> +
> (rel)->pgstat_info->counts.parallelnumscans++;
> + }
>
> I think counts.numscans has to be incremented in all the cases (so even if
> "parallel" is true).
>
> Same comment for pgstat_count_index_scan().
>
>
You're right, and I've been too quick. Fixed in v3.
> > 4 ===
> > >
> > > + if (lstats->counts.numscans || lstats->counts.parallelnumscans)
> > >
> > > Is it possible to have (lstats->counts.parallelnumscans) whithout
> having
> > > (lstats->counts.numscans) ?
> > >
> > >
> > Nope, parallel scans are included in seq/index scans, as far as I can
> tell.
> > I could remove the parallelnumscans testing but it would be less obvious
> to
> > read.
>
> 2 ===
>
> What about adding a comment instead of this extra check?
>
>
Done too in v3.
--
Guillaume.
From 97a95650cd220c1b88ab6f3d36149b8860bead1d Mon Sep 17 00:00:00 2001
From: Guillaume Lelarge
Date: Wed, 28 Aug 2024 21:35:30 +0200
Subject: [PATCH v3] Add parallel columns for pg_stat_all_tables,indexes
pg_stat_all_tables gets 4 new columns: parallel_seq_scan,
last_parallel_seq_scan, parallel_idx_scan, last_parallel_idx_scan.
pg_stat_all_indexes gets 2 new columns: parallel_idx_scan,
last_parallel_idx_scan.
---
doc/src/sgml/monitoring.sgml | 69 ++--
src/backend/access/brin/brin.c | 2 +-
src/backend/access/gin/ginscan.c | 2 +-
src/backend/access/gist/gistget.c| 4 +-
src/backend/access/hash/hashsearch.c | 2 +-
src/backend/access/heap/heapam.c | 2 +-
src/backend/access/nbtree/nbtsearch.c| 2 +-
src/backend/access/spgist/spgscan.c | 2 +-
src/backend/catalog/system_views.sql | 6 ++
src/backend/utils/activity/pgstat_relation.c | 10 ++-
src/backend/utils/adt/pgstatfuncs.c | 6 ++
src/include/catalog/pg_proc.dat | 8 +++
src/include/pgstat.h | 17 +++--
13 files changed, 113 insertions(+), 19 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 933de6fe07..6886094095 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3773,7 +3773,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage
seq_scan bigint
- Number of sequential scans initiated on this table
+ Number of sequential scans (including parallel ones) initiated on this table
@@ -3782,7 +3782,26 @@ description | Waiting for a newly initialized WAL file to reach durable storage
last_seq_scan timestamp with time zone
- The time of the last sequential scan on this table, based on the
+ The time of the last sequential scan (including parallel ones) on this table, based on the
+ most recent transaction stop time
+
+
+
+
+
+ parallel_seq_scan bigint
+
+
+ Number of parallel sequential scans initiated on this table
+
+
+
+
+
+ last_paralle
Re: Add parallel columns for seq scan and index scan on pg_stat_all_tables and _indexes
Hi, Le mer. 4 sept. 2024 à 10:47, Bertrand Drouvot a écrit : > Hi, > > On Thu, Aug 29, 2024 at 04:04:05PM +0200, Guillaume Lelarge wrote: > > Hello, > > > > This patch was a bit discussed on [1], and with more details on [2]. It > > introduces four new columns in pg_stat_all_tables: > > > > * parallel_seq_scan > > * last_parallel_seq_scan > > * parallel_idx_scan > > * last_parallel_idx_scan > > > > and two new columns in pg_stat_all_indexes: > > > > * parallel_idx_scan > > * last_parallel_idx_scan > > > > As Benoit said yesterday, the intent is to help administrators evaluate > the > > usage of parallel workers in their databases and help configuring > > parallelization usage. > > Thanks for the patch. I think that's a good idea to provide more > instrumentation > in this area. So, +1 regarding this patch. > > Thanks. > A few random comments: > > 1 === > > + > + > + parallel_seq_scan bigint > + > + > + Number of parallel sequential scans initiated on this table > + > + > > I wonder if we should not update the seq_scan too to indicate that it > includes the parallel_seq_scan. > > Same kind of comment for last_seq_scan, idx_scan and last_idx_scan. > > Yeah, not sure why I didn't do it at first. I was wondering the same thing. The patch attached does this. > 2 === > > @@ -410,6 +410,8 @@ initscan(HeapScanDesc scan, ScanKey key, bool > keep_startblock) > */ > if (scan->rs_base.rs_flags & SO_TYPE_SEQSCAN) > pgstat_count_heap_scan(scan->rs_base.rs_rd); > + if (scan->rs_base.rs_parallel != NULL) > + pgstat_count_parallel_heap_scan(scan->rs_base.rs_rd); > > Indentation seems broken. > > My bad, sorry. Fixed in the attached patch. > Shouldn't the parallel counter relies on the "scan->rs_base.rs_flags & > SO_TYPE_SEQSCAN" > test too? > > You're right. Fixed in the attached patch. > What about to get rid of the pgstat_count_parallel_heap_scan and add an > extra > bolean parameter to pgstat_count_heap_scan to indicate if > counts.parallelnumscans > should be incremented too? > > Something like: > > pgstat_count_heap_scan(scan->rs_base.rs_rd, scan->rs_base.rs_parallel != > NULL) > > 3 === > > Same comment for pgstat_count_index_scan (add an extra bolean parameter) > and > get rid of pgstat_count_parallel_index_scan()). > > I think that 2 === and 3 === would help to avoid missing increments should > we > add those call to other places in the future. > > Oh OK, understood. Done for both. 4 === > > + if (lstats->counts.numscans || lstats->counts.parallelnumscans) > > Is it possible to have (lstats->counts.parallelnumscans) whithout having > (lstats->counts.numscans) ? > > Nope, parallel scans are included in seq/index scans, as far as I can tell. I could remove the parallelnumscans testing but it would be less obvious to read. > > First time I had to add new columns to a statistics catalog. I'm actually > > not sure that we were right to change pg_proc.dat manually. > > I think that's the right way to do. > > OK, new patch attached. > I don't see a CF entry for this patch. Would you mind creating one so that > we don't lost track of it? > > I don't mind adding it, though I don't know if I should add it to the September or November commit fest. Which one should I choose? Thanks. Regards. -- Guillaume. From 6a202b7bd44cf33be13a8f7e0a8dc7077604c3c0 Mon Sep 17 00:00:00 2001 From: Guillaume Lelarge Date: Wed, 28 Aug 2024 21:35:30 +0200 Subject: [PATCH v2] Add parallel columns for pg_stat_all_tables,indexes pg_stat_all_tables gets 4 new columns: parallel_seq_scan, last_parallel_seq_scan, parallel_idx_scan, last_parallel_idx_scan. pg_stat_all_indexes gets 2 new columns: parallel_idx_scan, last_parallel_idx_scan. --- doc/src/sgml/monitoring.sgml | 69 ++-- src/backend/access/brin/brin.c | 2 +- src/backend/access/gin/ginscan.c | 2 +- src/backend/access/gist/gistget.c| 4 +- src/backend/access/hash/hashsearch.c | 2 +- src/backend/access/heap/heapam.c | 2 +- src/backend/access/nbtree/nbtsearch.c| 2 +- src/backend/access/spgist/spgscan.c | 2 +- src/backend/catalog/system_views.sql | 6 ++ src/backend/utils/activity/pgstat_relation.c | 7 +- src/backend/utils/adt/pgstatfuncs.c | 6 ++ src/include/catalog/pg_proc.dat | 8 +++ src/include/pgstat.h | 23 +-- 13 fil
Re: Add parallel columns for seq scan and index scan on pg_stat_all_tables and _indexes
Le jeu. 5 sept. 2024 à 07:36, Bertrand Drouvot a écrit : > Hi, > > On Wed, Sep 04, 2024 at 04:37:19PM +0200, Guillaume Lelarge wrote: > > Hi, > > > > Le mer. 4 sept. 2024 à 16:18, Bertrand Drouvot < > bertranddrouvot...@gmail.com> > > a écrit : > > > What about adding a comment instead of this extra check? > > > > > > > > Done too in v3. > > Thanks! > > 1 === > > + /* > +* Don't check counts.parallelnumscans because counts.numscans > includes > +* counts.parallelnumscans > +*/ > > "." is missing at the end of the comment. > > Fixed in v4. > 2 === > > - if (t > tabentry->lastscan) > + if (t > tabentry->lastscan && lstats->counts.numscans) > > The extra check on lstats->counts.numscans is not needed as it's already > done > a few lines before. > > Fixed in v4. > 3 === > > + if (t > tabentry->parallellastscan && > lstats->counts.parallelnumscans) > > This one makes sense. > > And now I'm wondering if the extra comment added in v3 is really worth it > (and > does not sound confusing)? I mean, the parallel check is done once we passe > the initial test on counts.numscans. I think the code is clear enough > without > this extra comment, thoughts? > > I'm not sure I understand you here. I kinda like the extra comment though. > 4 === > > What about adding a few tests? or do you want to wait a bit more to see if > " > there's an agreement on this patch" (as you stated at the start of this > thread). > > Guess I can start working on that now. It will take some time as I've never done it before. Good thing I added the patch on the November commit fest :) Thanks again. Regards. -- Guillaume. From 6c92e70cd2698f24fe14069f675b7934e2f95bfe Mon Sep 17 00:00:00 2001 From: Guillaume Lelarge Date: Wed, 28 Aug 2024 21:35:30 +0200 Subject: [PATCH v4] Add parallel columns for pg_stat_all_tables,indexes pg_stat_all_tables gets 4 new columns: parallel_seq_scan, last_parallel_seq_scan, parallel_idx_scan, last_parallel_idx_scan. pg_stat_all_indexes gets 2 new columns: parallel_idx_scan, last_parallel_idx_scan. --- doc/src/sgml/monitoring.sgml | 69 ++-- src/backend/access/brin/brin.c | 2 +- src/backend/access/gin/ginscan.c | 2 +- src/backend/access/gist/gistget.c| 4 +- src/backend/access/hash/hashsearch.c | 2 +- src/backend/access/heap/heapam.c | 2 +- src/backend/access/nbtree/nbtsearch.c| 2 +- src/backend/access/spgist/spgscan.c | 2 +- src/backend/catalog/system_views.sql | 6 ++ src/backend/utils/activity/pgstat_relation.c | 8 +++ src/backend/utils/adt/pgstatfuncs.c | 6 ++ src/include/catalog/pg_proc.dat | 8 +++ src/include/pgstat.h | 17 +++-- 13 files changed, 112 insertions(+), 18 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 933de6fe07..6886094095 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -3773,7 +3773,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage seq_scan bigint - Number of sequential scans initiated on this table + Number of sequential scans (including parallel ones) initiated on this table @@ -3782,7 +3782,26 @@ description | Waiting for a newly initialized WAL file to reach durable storage last_seq_scan timestamp with time zone - The time of the last sequential scan on this table, based on the + The time of the last sequential scan (including parallel ones) on this table, based on the + most recent transaction stop time + + + + + + parallel_seq_scan bigint + + + Number of parallel sequential scans initiated on this table + + + + + + last_parallel_seq_scan timestamp with time zone + + + The time of the last parallel sequential scan on this table, based on the most recent transaction stop time @@ -3801,7 +3820,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage idx_scan bigint - Number of index scans initiated on this table + Number of index scans (including parallel ones) initiated on this table @@ -3810,7 +3829,26 @@ description | Waiting for a newly initialized WAL file to reach durable storage last_idx_scan timestamp with time zone - The time of the last index sca
