Re: Split xlog.c

2022-02-15 Thread Heikki Linnakangas
On 14/02/2022 11:36, Heikki Linnakangas wrote: On 27/01/2022 08:34, Michael Paquier wrote: On Tue, Jan 25, 2022 at 12:12:40PM +0200, Heikki Linnakangas wrote: In last round of review, I spotted one bug: I had mixed up the meaning of EndOfLogTLI. It is the TLI in the *filename* of the WAL

Re: some aspects of our qsort might not be ideal

2022-02-15 Thread John Naylor
> This way, we *dynamically* > decide to optimistically start an insertion sort, in the hopes that it > will occasionally prevent a recursion, and worst case the input is > slightly more sorted for the next recursion. I should mention one detail -- we put a limit on how many iterations we attempt

Re: BufferAlloc: don't take two simultaneous locks

2022-02-15 Thread Yura Sokolov
Hello, all. I thought about patch simplification, and tested version without BufTable and dynahash api change at all. It performs suprisingly well. It is just a bit worse than v1 since there is more contention around dynahash's freelist, but most of improvement remains. I'll finish benchmarking

Re: some aspects of our qsort might not be ideal

2022-02-15 Thread John Naylor
> [3] > https://www.postgresql.org/message-id/flat/87F42982BF2B434F831FCEF4C45FC33E5BD369DF%40EXCHANGE.corporate.connx.com#e69718293c45d89555020bd0922ad055 The top of that thread is where I meant to point to:

Re: BufferAlloc: don't take two simultaneous locks

2022-02-15 Thread Yura Sokolov
В Вс, 06/02/2022 в 19:34 +0300, Michail Nikolaev пишет: > Hello, Yura. > > A one additional moment: > > > 1332: Assert((oldFlags & (BM_PIN_COUNT_WAITER | BM_IO_IN_PROGRESS)) == 0); > > 1333: CLEAR_BUFFERTAG(buf->tag); > > 1334: buf_state &= ~(BUF_FLAG_MASK | BUF_USAGECOUNT_MASK); > > 1335:

some aspects of our qsort might not be ideal

2022-02-15 Thread John Naylor
While reviewing Thomas Munro's patchset to consider expanding the uses of specialized qsort [1], I wondered about some aspects of the current qsort implementation. For background, ours is based on Bentley & McIlroy "Engineering a Sort Function" [2], which is a classic paper worth studying. 1) the

Re: Trap errors from streaming child in pg_basebackup to exit early

2022-02-15 Thread Michael Paquier
On Wed, Sep 29, 2021 at 01:18:40PM +0200, Daniel Gustafsson wrote: > So there is one mention of a background WAL receiver already in there, but > it's > pretty inconsistent as to what we call it. For now I've changed the messaging > in this patch to say "background process", leaving making this

Re: Observability in Postgres

2022-02-15 Thread Julien Rouhaud
Hi, On Mon, Feb 14, 2022 at 03:15:14PM -0500, Greg Stark wrote: > > [...] > 2) SQL connections are tied to specific databases within a cluster. > Making it hard to get data for all your databases if you have more > than one. The exporter needs to reconnect to each database. > > 3) The exporter

Re: Race conditions in 019_replslot_limit.pl

2022-02-15 Thread Kyotaro Horiguchi
At Wed, 16 Feb 2022 15:22:27 +0900 (JST), Kyotaro Horiguchi wrote in > At Wed, 16 Feb 2022 14:58:23 +0900, Masahiko Sawada > wrote in > > Or it's possible that the process took a time to clean up the > > temporary replication slot? > > Checkpointer may take ReplicationSlotControlLock. Dead

Re: Observability in Postgres

2022-02-15 Thread Greg Stark
On Tue, 15 Feb 2022 at 17:37, Magnus Hagander wrote: > > On Tue, Feb 15, 2022 at 11:24 PM Greg Stark wrote: > > > > On Tue, 15 Feb 2022 at 16:43, Magnus Hagander wrote: > > I really don't see the problem with having the monitoring on a different port. > > I *do* see the problem with having a

Re: USE_BARRIER_SMGRRELEASE on Linux?

2022-02-15 Thread Nathan Bossart
On Tue, Feb 15, 2022 at 10:14:04PM -0800, Nathan Bossart wrote: > It looks like register_unlink_segment() is called prior to the checkpoint, > but the checkpointer is not calling RememberSyncRequest() until after > SyncPreCheckpoint(). This means that the requests are registered with the > next

Re: Adding CI to our tree

2022-02-15 Thread Andres Freund
Hi, On February 15, 2022 10:12:36 PM PST, Justin Pryzby wrote: >On Sun, Feb 13, 2022 at 01:53:19PM -0800, Andres Freund wrote: >> Hi, >> >> On 2022-02-13 15:31:20 -0600, Justin Pryzby wrote: >> > Oh - I suppose you're right. That's an unfortunate consequence of running >> > a >> > single

Re: Race conditions in 019_replslot_limit.pl

2022-02-15 Thread Kyotaro Horiguchi
At Wed, 16 Feb 2022 14:58:23 +0900, Masahiko Sawada wrote in > Or it's possible that the process took a time to clean up the > temporary replication slot? Checkpointer may take ReplicationSlotControlLock. Dead lock between ReplicationSlotCleanup and InvalidateObsoleteReplicationSlots happened?

Re: USE_BARRIER_SMGRRELEASE on Linux?

2022-02-15 Thread Nathan Bossart
On Tue, Feb 15, 2022 at 09:06:45PM -0800, Andres Freund wrote: > On 2022-02-15 20:59:11 -0800, Nathan Bossart wrote: >> On Wed, Feb 16, 2022 at 03:34:08PM +1300, Thomas Munro wrote: >> > So it's not getting unlinked until the *next* checkpoint cycle. I >> > don't know why. >> >> On my machine

Re: Adding CI to our tree

2022-02-15 Thread Justin Pryzby
On Sun, Feb 13, 2022 at 01:53:19PM -0800, Andres Freund wrote: > Hi, > > On 2022-02-13 15:31:20 -0600, Justin Pryzby wrote: > > Oh - I suppose you're right. That's an unfortunate consequence of running a > > single prove instance without chdir. > > I don't think it's chdir that's relevant (that

Re: Race conditions in 019_replslot_limit.pl

2022-02-15 Thread Kyotaro Horiguchi
At Wed, 16 Feb 2022 14:58:23 +0900, Masahiko Sawada wrote in > On Wed, Feb 16, 2022 at 2:26 PM Kyotaro Horiguchi > wrote: > > > > At Tue, 15 Feb 2022 15:51:57 -0800, Andres Freund > > wrote in > > > I guess it's conceivable that the backend was still working through > > > process > > >

Re: Race conditions in 019_replslot_limit.pl

2022-02-15 Thread Kyotaro Horiguchi
At Wed, 16 Feb 2022 14:26:37 +0900 (JST), Kyotaro Horiguchi wrote in > Agreed. Doing this att all slot creation seems fine. Done in the attached. The first slot is deliberately created unreserved so I changed the code to re-create the slot with "reserved" before taking backup. > > Even

Re: Race conditions in 019_replslot_limit.pl

2022-02-15 Thread Masahiko Sawada
On Wed, Feb 16, 2022 at 2:26 PM Kyotaro Horiguchi wrote: > > At Tue, 15 Feb 2022 15:51:57 -0800, Andres Freund wrote > in > > I think what happened is that there was no WAL to receive between the start > > of > > the primary and the $node_primary3->wait_for_catchup($node_standby3); > > > >

Re: make tuplestore helper function

2022-02-15 Thread Justin Pryzby
On Tue, Jan 11, 2022 at 10:19:33AM -0500, Melanie Plageman wrote: > > If the expectedDesc NULL check were an 0001 patch, then 0002 (the main > > patch) > > would be even easier to review. Only foreign.c is different. > > I'll wait to do that if preferred by committer. > Are you imagining that

Re: Observability in Postgres

2022-02-15 Thread Greg Stark
On Tue, 15 Feb 2022 at 22:48, Stephan Doliov wrote: > > I am curious what you mean by standard metrics format? I am all for > standards-based but what are those in the case of DBs. For environments where > O11y matters a lot, I think the challenge lies in mapping specific query > executions

Re: make tuplestore helper function

2022-02-15 Thread Michael Paquier
On Tue, Jan 11, 2022 at 10:19:33AM -0500, Melanie Plageman wrote: > On Wed, Jan 5, 2022 at 7:57 PM Justin Pryzby wrote: >> I'd leave it for input from a committer about those: >> >> - remove tuplestore_donestoring() calls ? This part has been raised on a different thread, as of:

Re: Small and unaffected typo in pg_logical_slot_get_changes_guts()

2022-02-15 Thread Michael Paquier
On Tue, Feb 15, 2022 at 11:23:29PM -0600, Justin Pryzby wrote: > On Wed, Feb 16, 2022 at 11:27:58AM +0900, Kasahara Tatsuhito wrote: >> - tuplestore_donestoring(tupstore); >> + tuplestore_donestoring(p->tupstore); > > Melanie's tuplestore patch also removes the bogus

Re: Race conditions in 019_replslot_limit.pl

2022-02-15 Thread Kyotaro Horiguchi
At Tue, 15 Feb 2022 15:51:57 -0800, Andres Freund wrote in > I think what happened is that there was no WAL to receive between the start of > the primary and the $node_primary3->wait_for_catchup($node_standby3); > > Because the slot is created without reserving WAL that allows the primary to >

Re: Small and unaffected typo in pg_logical_slot_get_changes_guts()

2022-02-15 Thread Justin Pryzby
On Wed, Feb 16, 2022 at 11:27:58AM +0900, Kasahara Tatsuhito wrote: > - tuplestore_donestoring(tupstore); > + tuplestore_donestoring(p->tupstore); Melanie's tuplestore patch also removes the bogus line.

Re: support for CREATE MODULE

2022-02-15 Thread Pavel Stehule
Hi > Yes, anything a user may want to do with modules is likely possible to > do already with schemas. But just because it is possible doesn't mean > it is not tedious and awkward because of the mechanisms available to do > them now. And I would advocate for more expressive constructs to enable

Re: USE_BARRIER_SMGRRELEASE on Linux?

2022-02-15 Thread Andres Freund
Hi, On 2022-02-15 20:59:11 -0800, Nathan Bossart wrote: > On Wed, Feb 16, 2022 at 03:34:08PM +1300, Thomas Munro wrote: > > So it's not getting unlinked until the *next* checkpoint cycle. I > > don't know why. > > On my machine (5.11.0-43), it looks like the test starts failing after > cc50080.

Proposal for internal Numeric to Uint64 conversion function.

2022-02-15 Thread Amul Sul
Hi, Currently, numeric_pg_lsn is the only one that accepts the Numeric value and converts to uint64 and that is the reason all the type conversion code is embedded into it. I think it would be helpful if that numeric-to-uint64 conversion code is extracted as a separate function so that any other

Re: USE_BARRIER_SMGRRELEASE on Linux?

2022-02-15 Thread Andres Freund
Hi, On 2022-02-16 15:34:08 +1300, Thomas Munro wrote: > So it's not getting unlinked until the *next* checkpoint cycle. I > don't know why. It might be helpful to know what the relfilenode maps to, so we know the operations done to it. Perhaps logging in heap_create() and

Re: USE_BARRIER_SMGRRELEASE on Linux?

2022-02-15 Thread Nathan Bossart
On Wed, Feb 16, 2022 at 03:34:08PM +1300, Thomas Munro wrote: > So it's not getting unlinked until the *next* checkpoint cycle. I > don't know why. On my machine (5.11.0-43), it looks like the test starts failing after cc50080. That commit adjusted some regression tests, so I'm assuming it's

Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set

2022-02-15 Thread Michael Paquier
On Tue, Feb 15, 2022 at 01:02:41PM +0900, Michael Paquier wrote: > Hmm. At the end of the day, I am wondering whether we should not give > up entirely on the concept of running the regression tests on older > branches in the TAP script of a newer branch. pg_regress needs to > come from the old

Re: Small and unaffected typo in pg_logical_slot_get_changes_guts()

2022-02-15 Thread Kasahara Tatsuhito
Hi ! 2022年2月16日(水) 11:42 Michael Paquier : > > On Wed, Feb 16, 2022 at 11:27:58AM +0900, Kasahara Tatsuhito wrote: > > Since commit dd04e958c8b03c0f0512497651678c7816af3198, > > tuplestore_donestoring() seems to be left only for compatibility, so > > it looks like it can be removed from the core

Re: USE_BARRIER_SMGRRELEASE on Linux?

2022-02-15 Thread Andres Freund
Hi, On 2022-02-16 15:09:15 +1300, Thomas Munro wrote: > I pushed a hack to log the name of the file that's getting in the way: FWIW, I think we should just have that in core. Right now it's just pointlessly hard to debug. I think just reporting the first file would be good enough... Greetings,

Re: pg_upgrade verbosity when redirecting output to log file

2022-02-15 Thread Thomas Munro
On Tue, Jan 11, 2022 at 4:42 AM Bruce Momjian wrote: > On Sun, Jan 9, 2022 at 10:39:58PM -0800, Andres Freund wrote: > > On 2022-01-10 01:14:32 -0500, Tom Lane wrote: > > > I think I'd vote for just nuking that output altogether. > > > It seems of very dubious value. > > > > It seems worthwhile

Re: Observability in Postgres

2022-02-15 Thread Stephan Doliov
I am curious what you mean by standard metrics format? I am all for standards-based but what are those in the case of DBs. For environments where O11y matters a lot, I think the challenge lies in mapping specific query executions back to system characteristics. I am just thinking aloud as a newbie

Re: pg_walinspect - a new extension to get raw WAL data and WAL stats

2022-02-15 Thread Ashutosh Sharma
On Wed, Feb 16, 2022 at 1:01 AM Bharath Rupireddy wrote: > > On Mon, Feb 14, 2022 at 8:32 PM Ashutosh Sharma wrote: > > > > Here are few comments: > > Thanks for reviewing the patches. > > > +/* > > + * Verify the authenticity of the given raw WAL record. > > + */ > > +Datum > >

Re: pg_walinspect - a new extension to get raw WAL data and WAL stats

2022-02-15 Thread Bharath Rupireddy
On Wed, Feb 16, 2022 at 1:57 AM Robert Haas wrote: > > On Tue, Feb 15, 2022 at 2:31 PM Bharath Rupireddy > wrote: > > > +/* > > > + * Verify the authenticity of the given raw WAL record. > > > + */ > > > +Datum > > > +pg_verify_raw_wal_record(PG_FUNCTION_ARGS) > > > +{ > > > > > > > > > Do we

RE: logical replication empty transactions

2022-02-15 Thread osumi.takami...@fujitsu.com
Hi I'll quote one other remaining discussion of this thread again to invoke more attentions from the community. On Friday, August 13, 2021 8:01 PM Ajin Cherian wrote: > On Mon, Aug 2, 2021 at 7:20 PM Amit Kapila > wrote: > > Few other miscellaneous comments: > > 1. > > static void > >

Re: initdb / bootstrap design

2022-02-15 Thread John Naylor
On Wed, Feb 16, 2022 at 9:12 AM Andres Freund wrote: > To me the division of labor between initdb and bootstrap doesn't make much > sense anymore: [...] > I don't plan to work on this immediately, but I thought it's worth bringing up > anyway. Added TODO item "Rationalize division of labor

Re: [PATCH] Fix out-of-bouds access (src/common/wchar.c)

2022-02-15 Thread Kyotaro Horiguchi
At Tue, 15 Feb 2022 09:17:34 -0300, Ranier Vilela wrote in > Per Coverity. Thanks for the source:) > Like the function pg_encoding_max_length_sql > (src/backend/utils/mb/mbutils.c) > Only assertion is insufficient to avoid accessing array out-of-bounds. > > This bug is live according

Re: [Proposal] Add foreign-server health checks infrastructure

2022-02-15 Thread Kyotaro Horiguchi
At Tue, 1 Feb 2022 23:51:54 +0900, Fujii Masao wrote in > This logic sounds complicated to me. I'm afraid that FDW developers > may a bit easily misunderstand the logic and make the bug in their > FDW. > Isn't it simpler to just disable the timeout in core whenever the > transaction ends

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Andres Freund
Hi,m On 2022-02-15 20:56:15 -0500, Tom Lane wrote: > Maybe we have a bit more flexibility for TOAST, not sure. It'd be nice to at least add it as an option for initdb. Afaics there's no way to change the default at that point. initdb itself is measurably faster. Although sadly it's a bigger

Re: Small and unaffected typo in pg_logical_slot_get_changes_guts()

2022-02-15 Thread Michael Paquier
On Wed, Feb 16, 2022 at 11:27:58AM +0900, Kasahara Tatsuhito wrote: > Since commit dd04e958c8b03c0f0512497651678c7816af3198, > tuplestore_donestoring() seems to be left only for compatibility, so > it looks like it can be removed from the core code, except for the > header (tuplestore.h). FWIW,

Re: USE_BARRIER_SMGRRELEASE on Linux?

2022-02-15 Thread Thomas Munro
On Wed, Feb 16, 2022 at 3:09 PM Thomas Munro wrote: > I tried adding another log line so I could see the unlink() happening, > but with that it doesn't fail (though I'm still trying). We may be in > heisenbug territory. This time I was luckier:

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Michael Paquier
On Tue, Feb 15, 2022 at 02:33:32PM -0600, Justin Pryzby wrote: > What I mean is that any patches which *use* the zstd support should update > those for completeness. > > doc/src/sgml/config.sgml | 14 +- > doc/src/sgml/install-windows.sgml | 2 +- >

Small and unaffected typo in pg_logical_slot_get_changes_guts()

2022-02-15 Thread Kasahara Tatsuhito
Hi, I noticed the small typo in pg_logical_slot_get_changes_guts(). == --- a/src/backend/replication/logical/logicalfuncs.c +++ b/src/backend/replication/logical/logicalfuncs.c @@ -295,7 +295,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Andres Freund
Hi, On 2022-02-16 09:58:44 +0900, Michael Paquier wrote: > On Tue, Feb 15, 2022 at 01:42:55PM -0800, Andres Freund wrote: > > I think well before removing stuff we should default to decent compression > > algorithms. E.g. -Z/--compress should probably not continue to use gzip if > > better things

Re: pgsql: Add TAP test to automate the equivalent of check_guc

2022-02-15 Thread Michael Paquier
On Mon, Feb 14, 2022 at 02:11:37PM +0900, Michael Paquier wrote: > A run through the CI shows that this is working, and it also works > with Debian's patch for the config data. I still tend to prefer the > readability of a TAP test over the main regression test suite for this > facility. Hearing

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-15 Thread John Naylor
On Tue, Feb 15, 2022 at 11:07 PM Andres Freund wrote: > > diff --git a/src/include/common/relpath.h b/src/include/common/relpath.h > > index a4b5dc853b..13849a3790 100644 > > --- a/src/include/common/relpath.h > > +++ b/src/include/common/relpath.h > > @@ -56,7 +56,7 @@ typedef enum ForkNumber >

initdb / bootstrap design

2022-02-15 Thread Andres Freund
Hi, [1] reminded me of a topic that I wanted to bring up at some point: To me the division of labor between initdb and bootstrap doesn't make much sense anymore: initdb reads postgres.bki, replaces a few tokens, starts postgres in bootstrap mode, and then painstakenly feeds bootstrap.bki lines

Re: USE_BARRIER_SMGRRELEASE on Linux?

2022-02-15 Thread Thomas Munro
Hi Nathan, On Wed, Feb 16, 2022 at 12:58 PM Nathan Bossart wrote: > I've noticed that my WAL pre-allocation patches [0] routinely fail with > "tablespace is not empty" errors on Linux [1]: > > DROP TABLESPACE regress_tblspace_renamed; > +ERROR: tablespace

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Tom Lane
Robert Haas writes: > On Tue, Feb 15, 2022 at 7:09 PM David Steele wrote: >> This is even more true for zstd since it is not as ubiquitous as lz4. In >> fact, it is not even included with base RHEL8. You need to install EPEL >> to get zstd. > Yeah, I agree. One thing I was thinking about but

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Robert Haas
On Tue, Feb 15, 2022 at 7:09 PM David Steele wrote: > I'm not sure that this is entirely true. lz4 is available on most > non-EOL distros but you don't need to look to far to find a distro that > does not have it. So, choosing lz4 by default could impact binary > portability. Of course, there are

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Michael Paquier
On Tue, Feb 15, 2022 at 01:42:55PM -0800, Andres Freund wrote: > I think well before removing stuff we should default to decent compression > algorithms. E.g. -Z/--compress should probably not continue to use gzip if > better things are available. Which one of lz4 or zstd would it be though? LZ4

Re: Column Filtering in Logical Replication

2022-02-15 Thread Tomas Vondra
On 2/16/22 01:33, Alvaro Herrera wrote: > On 2022-Feb-16, Tomas Vondra wrote: > >> Here's an updated version of the patch, rebased to current master. Parts >> 0002 and 0003 include various improvements based on review by me and another >> one by Peter Smith [1]. > > Thanks for doing this! >

Re: Column Filtering in Logical Replication

2022-02-15 Thread Alvaro Herrera
On 2022-Feb-16, Tomas Vondra wrote: > Here's an updated version of the patch, rebased to current master. Parts > 0002 and 0003 include various improvements based on review by me and another > one by Peter Smith [1]. Thanks for doing this! > 1) partitioning with pubviaroot=true I agree that

Fix a comment in worker.c

2022-02-15 Thread Masahiko Sawada
Hi, While reading the code, I realized that the second sentence of the following comment in worker.c is not correct: /* * Exit if the subscription was disabled. This normally should not happen * as the worker gets killed during ALTER SUBSCRIPTION ... DISABLE. */ if

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread David Steele
On 2/15/22 16:10, Peter Geoghegan wrote: On Tue, Feb 15, 2022 at 12:00 PM Robert Haas wrote: I'm not sure I completely follow this. There are cases where we use compression algorithms for internal purposes, and we can change the defaults without users knowing or caring. For example, we could

USE_BARRIER_SMGRRELEASE on Linux?

2022-02-15 Thread Nathan Bossart
I've noticed that my WAL pre-allocation patches [0] routinely fail with "tablespace is not empty" errors on Linux [1]: DROP TABLESPACE regress_tblspace_renamed; +ERROR: tablespace "regress_tblspace_renamed" is not empty This appears to have been discussed elsewhere (and fixed)

Re: Column Filtering in Logical Replication

2022-02-15 Thread Tomas Vondra
Hi Peter, Thanks for the review and sorry for taking so long. I've addressed most of the comments in the patch I sent a couple minutes ago. More comments in-line: On 1/28/22 09:39, Peter Smith wrote: Here are some review comments for the v17-0001 patch. ~~~ 1. Commit message If no

Re: Race conditions in 019_replslot_limit.pl

2022-02-15 Thread Andres Freund
Hi, On 2022-02-15 23:29:20 +0200, Heikki Linnakangas wrote: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=desmoxytes=2022-02-14%2006%3A30%3A04 > > [07:42:23] t/018_wal_optimize.pl ok12403 ms ( 0.00 usr > 0.00 sys + 1.40 cusr 0.63 csys = 2.03 CPU) > #

Re: last_archived_wal is not necessary the latest WAL file (was Re: pgsql: Add test case for an archive recovery corner case.)

2022-02-15 Thread Heikki Linnakangas
On 15/02/2022 23:28, Tom Lane wrote: Heikki Linnakangas writes: That was interesting: the order that WAL segments are archived when a standby is promoted is not fully deterministic. Oh, of course. I find it a bit surprising that pg_stat_archiver.last_archived_wal is not necessarily the

Re: do only critical work during single-user vacuum?

2022-02-15 Thread Peter Geoghegan
On Tue, Feb 15, 2022 at 9:28 AM Peter Geoghegan wrote: > On Mon, Feb 14, 2022 at 10:04 PM John Naylor > wrote: > > Well, the point of inventing this new vacuum mode was because I > > thought that upon reaching xidStopLimit, we couldn't issue commands, > > period, under the postmaster. If it was

Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work

2022-02-15 Thread Nathan Bossart
On Tue, Feb 15, 2022 at 10:37:58AM -0800, Nathan Bossart wrote: > On Tue, Feb 15, 2022 at 10:10:34AM -0800, Andres Freund wrote: >> I generally think it'd be a good exercise to go through an use >> get_dirent_type() in nearly all ReadDir() style loops - it's a nice >> efficiency >> win in

Re: Observability in Postgres

2022-02-15 Thread Magnus Hagander
On Tue, Feb 15, 2022 at 11:24 PM Greg Stark wrote: > > On Tue, 15 Feb 2022 at 16:43, Magnus Hagander wrote: > > > > On Tue, Feb 15, 2022 at 1:30 PM Dave Page wrote: > > > > > > - Does it really matter if metrics are exposed on a separate port from > > > the postmaster? I actually think doing

Re: Observability in Postgres

2022-02-15 Thread Greg Stark
On Tue, 15 Feb 2022 at 16:43, Magnus Hagander wrote: > > On Tue, Feb 15, 2022 at 1:30 PM Dave Page wrote: > > > > - Does it really matter if metrics are exposed on a separate port from the > > postmaster? I actually think doing that is a good thing as it allows use of > > alternative listen

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Peter Geoghegan
On Tue, Feb 15, 2022 at 12:00 PM Robert Haas wrote: > I'm not sure I completely follow this. There are cases where we use > compression algorithms for internal purposes, and we can change the > defaults without users knowing or caring. For example, we could decide > that LZ4-compressing FPIs in

Re: fixing bookindex.html bloat

2022-02-15 Thread Andres Freund
On 2022-02-15 11:16:12 +0100, Peter Eisentraut wrote: > On 15.02.22 00:06, Andres Freund wrote: > > On 2022-02-14 23:06:20 +0100, Peter Eisentraut wrote: > > > For the xlink business, I don't have a better idea than you, so your > > > workaround proposal seems fine. > > > > K. Will you apply your

Re: Observability in Postgres

2022-02-15 Thread Magnus Hagander
On Tue, Feb 15, 2022 at 1:30 PM Dave Page wrote: > > Hi Greg, > > On Mon, 14 Feb 2022 at 20:16, Greg Stark wrote: >> >> So I've been dealing a lot with building and maintaining dashboards >> for (fleets of) Postgres servers. And it's a pain. I have a few >> strongly held ideas about where the

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Andres Freund
Hi, On 2022-02-15 15:09:37 -0500, Robert Haas wrote: > On Tue, Feb 15, 2022 at 2:54 PM Andres Freund wrote: > > There's a difference between downloading a source tarball of 1.5x the size, > > and 3x the decompression cost (made up numbers), because the cost of either > > is > > not a relevant

Race conditions in 019_replslot_limit.pl

2022-02-15 Thread Heikki Linnakangas
While looking at recent failures in the new 028_pitr_timelines.pl recovery test, I noticed that there have been a few failures in the buildfarm in the recoveryCheck phase even before that, in the 019_replslot_limit.pl test. For example:

Re: last_archived_wal is not necessary the latest WAL file (was Re: pgsql: Add test case for an archive recovery corner case.)

2022-02-15 Thread Tom Lane
Heikki Linnakangas writes: > That was interesting: the order that WAL segments are archived when a > standby is promoted is not fully deterministic. Oh, of course. > I find it a bit surprising that pg_stat_archiver.last_archived_wal is > not necessarily the highest-numbered segment that was

PGEventProcs must not be allowed to break libpq

2022-02-15 Thread Tom Lane
I've been fooling around with the duplicated-error-text issue discussed in [1], and I think I have a solution that is fairly bulletproof ... except that it cannot cope with this little gem at the bottom of PQgetResult: if (!res->events[i].proc(PGEVT_RESULTCREATE, ,

last_archived_wal is not necessary the latest WAL file (was Re: pgsql: Add test case for an archive recovery corner case.)

2022-02-15 Thread Heikki Linnakangas
On 14/02/2022 22:43, Heikki Linnakangas wrote: On 14/02/2022 16:41, Tom Lane wrote: Heikki Linnakangas writes: Add test case for an archive recovery corner case. hoverfly seems not to like this: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hoverfly=2022-02-14%2012%3A36%3A12

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Justin Pryzby
On Tue, Feb 15, 2022 at 03:23:30PM -0500, Robert Haas wrote: > On Tue, Feb 15, 2022 at 1:55 PM Justin Pryzby wrote: > > Regarding the patch: > > > > I suppose it should support windows, and whatever patches use zstd should > > update the install instructions. See 9ca40dcd4, 02a93e7ef, 4035cd5d4.

Re: support for CREATE MODULE

2022-02-15 Thread Swaha Miller
On Mon, Feb 14, 2022 at 4:58 PM Bruce Momjian wrote: > On Mon, Feb 14, 2022 at 07:42:21PM -0500, Bruce Momjian wrote: > > On Mon, Feb 14, 2022 at 03:23:07PM -0800, Swaha Miller wrote: > > > A prominent use case for grouping functions into modules would > > > be access control on the group as a

Re: pg_walinspect - a new extension to get raw WAL data and WAL stats

2022-02-15 Thread Robert Haas
On Tue, Feb 15, 2022 at 2:31 PM Bharath Rupireddy wrote: > > +/* > > + * Verify the authenticity of the given raw WAL record. > > + */ > > +Datum > > +pg_verify_raw_wal_record(PG_FUNCTION_ARGS) > > +{ > > > > > > Do we really need this function? I see that whenever the record is > > read, we

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Robert Haas
On Tue, Feb 15, 2022 at 1:55 PM Justin Pryzby wrote: > Regarding the patch: > > I suppose it should support windows, and whatever patches use zstd should > update the install instructions. See 9ca40dcd4, 02a93e7ef, 4035cd5d4. Thanks, this is helpful. To me, it looks like we need something

Re: WIP: System Versioned Temporal Table

2022-02-15 Thread Jean Baro
Would these best practices be applicable by PostgreSQL to help avoid breaking changes for temporal tables? https://blog.datomic.com/2017/01/the-ten-rules-of-schema-growth.html Thanks On Tue, Feb 15, 2022 at 5:08 PM Vik Fearing wrote: > On 1/24/22 00:16, Corey Huinker wrote: > >> - Table

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Robert Haas
On Tue, Feb 15, 2022 at 2:54 PM Andres Freund wrote: > There's a difference between downloading a source tarball of 1.5x the size, > and 3x the decompression cost (made up numbers), because the cost of either is > not a relevant factor. I think basebackups are a different story. To be clear, I'm

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Robert Haas
On Tue, Feb 15, 2022 at 2:40 PM Peter Geoghegan wrote: > On Tue, Feb 15, 2022 at 10:20 AM Robert Haas wrote: > > In general, deciding on new compression algorithms can feel a bit like > > debating the merits of vi vs. emacs, or one political party vs. > > another. > > Really? I don't get that

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Andres Freund
Hi, On 2022-02-15 11:40:20 -0800, Peter Geoghegan wrote: > On Tue, Feb 15, 2022 at 10:20 AM Robert Haas wrote: > > Likewise, I still download the .tar.gz version of anything > > that gives me that option, basically because I'm familiar with the > > format and it's easy for me to just carry on

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Peter Geoghegan
On Tue, Feb 15, 2022 at 10:20 AM Robert Haas wrote: > In general, deciding on new compression algorithms can feel a bit like > debating the merits of vi vs. emacs, or one political party vs. > another. Really? I don't get that impression myself. (That's not important, though.) > What I imagine

Re: pg_walinspect - a new extension to get raw WAL data and WAL stats

2022-02-15 Thread Bharath Rupireddy
On Mon, Feb 14, 2022 at 8:32 PM Ashutosh Sharma wrote: > > Here are few comments: Thanks for reviewing the patches. > +/* > + * Verify the authenticity of the given raw WAL record. > + */ > +Datum > +pg_verify_raw_wal_record(PG_FUNCTION_ARGS) > +{ > > > Do we really need this function? I see

Re: pgsql: Move scanint8() to numutils.c

2022-02-15 Thread Tom Lane
Robert Haas writes: > On Tue, Feb 15, 2022 at 10:39 AM Joe Conway wrote: >> scanint8() is exported, and this change breaks at least two extensions I >> maintain. > I don't have a particularly strong view on whether the underlying > change was a good idea here, but the breakage you're talking

Re: pgsql: Move scanint8() to numutils.c

2022-02-15 Thread Joe Conway
On 2/15/22 13:47, Robert Haas wrote: On Tue, Feb 15, 2022 at 10:39 AM Joe Conway wrote: (moving to hackers) I guess shame on me for not noticing the thread, but I don't see any discussion about the potential for breakage to external projects. scanint8() is exported, and this change breaks at

Re: adding 'zstd' as a compression algorithm

2022-02-15 Thread Justin Pryzby
I think the WAL patch (4035cd5d4) should support zstd if library support is added. A supplementary patch for that already exists. https://www.postgresql.org/message-id/ynqwd2gsmrnqw...@paquier.xyz There's also some in-progress patches: - Konstantin and Daniil have a patch to add libpq

Re: Lowering the ever-growing heap->pd_lower

2022-02-15 Thread Matthias van de Meent
On Thu, 2 Dec 2021 at 11:17, Daniel Gustafsson wrote: > > This thread has stalled, and the request for benchmark/test has gone > unanswered > so I'm marking this patch Returned with Feedback. Please feel free to > resubmit > this patch if it is picked up again. Well then, here we go. It took

Re: pgsql: Move scanint8() to numutils.c

2022-02-15 Thread Robert Haas
On Tue, Feb 15, 2022 at 10:39 AM Joe Conway wrote: > (moving to hackers) > > I guess shame on me for not noticing the thread, but I don't see any > discussion about the potential for breakage to external projects. > > scanint8() is exported, and this change breaks at least two extensions I >

Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work

2022-02-15 Thread Nathan Bossart
On Tue, Feb 15, 2022 at 10:10:34AM -0800, Andres Freund wrote: > I generally think it'd be a good exercise to go through an use > get_dirent_type() in nearly all ReadDir() style loops - it's a nice efficiency > win in general, and IIRC a particularly big one on windows. > > It'd probably be good

Re: Design of pg_stat_subscription_workers vs pgstats

2022-02-15 Thread Robert Haas
On Thu, Jan 27, 2022 at 12:46 AM Andres Freund wrote: > Only if either the user wants to drop all stats, or somehow knows the oids of > already dropped tables... If it's really true that we can end up storing data for dropped objects, I think that's not acceptable and needs to be fixed. I don't

Re: Design of pg_stat_subscription_workers vs pgstats

2022-02-15 Thread Andres Freund
Hi, On 2022-02-03 13:33:08 +0900, Masahiko Sawada wrote: > I see that important information such as error-XID that can be used > for ALTER SUBSCRIPTION SKIP needs to be stored in a reliable way, and > using system catalogs is a reasonable way for this purpose. But it's > still unclear to me why

adding 'zstd' as a compression algorithm

2022-02-15 Thread Robert Haas
Hi, Over on the rather-long "refactoring basebackup.c" thread, there is a proposal, which I endorse, to add base-backup compression via zstd. To do that, we'd need to patch configure.ac to create a new --with-zstd flag and appropriate supporting infrastructure. My colleague Jeevan Ladhe included

Re: Design of pg_stat_subscription_workers vs pgstats

2022-02-15 Thread Andres Freund
Hi, On 2022-02-04 09:23:06 +0530, Amit Kapila wrote: > On Thu, Feb 3, 2022 at 3:25 PM Peter Eisentraut > wrote: > > > > On 02.02.22 07:54, Amit Kapila wrote: > > > > > Sure, but is this the reason you want to store all the error info in > > > the system catalog? I agree that providing more error

Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work

2022-02-15 Thread Andres Freund
Hi, On 2022-02-15 09:57:53 -0800, Nathan Bossart wrote: > IIUC you are advocating for something more like the attached patches. At least for patch 1 yes. Don't have the cycles just now to look at the others. I generally think it'd be a good exercise to go through an use get_dirent_type() in

Re: refactoring basebackup.c

2022-02-15 Thread Jeevan Ladhe
Thanks Tushar for the testing. I further worked on ZSTD and now have implemented client side compression as well. Attached are the patches for both server-side and client-side compression. The patch 0001 is a server-side patch, and has not changed since the last patch version - v10, but, just

Re: refactoring basebackup.c (zstd)

2022-02-15 Thread Justin Pryzby
+++ b/configure @@ -801,6 +805,7 @@ infodir docdir oldincludedir includedir +runstatedir There's superfluous changes to ./configure unrelated to the changes in configure.ac. Probably because you're using a different version of autotools, or a vendor's patched copy. You can remove the

Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work

2022-02-15 Thread Nathan Bossart
On Tue, Feb 15, 2022 at 09:09:52AM -0800, Andres Freund wrote: > On 2022-02-10 21:30:45 +0530, Bharath Rupireddy wrote: >> Replace ReadDir with ReadDirExtended (in CheckPointSnapBuild) and >> get rid of lstat entirely. > > I think this might be based on a slight misunderstanding / bad phrasing on

Re: Refactor CheckpointWriteDelay()

2022-02-15 Thread Nitin Jadhav
> Given that CheckpointWriteDelay() is really a hot code path i.e. gets > called for every dirty buffer written to disk, here's an opportunity > for us to improve it by avoiding some function calls. I have reviewed the patch and I agree that the patch improves the code. > Firstly, the

Re: do only critical work during single-user vacuum?

2022-02-15 Thread Peter Geoghegan
On Mon, Feb 14, 2022 at 10:04 PM John Naylor wrote: > Well, the point of inventing this new vacuum mode was because I > thought that upon reaching xidStopLimit, we couldn't issue commands, > period, under the postmaster. If it was easier to get a test instance > to xidStopLimit, I certainly would

Re: Mark all GUC variable as PGDLLIMPORT

2022-02-15 Thread Julien Rouhaud
Hi, Sorry for taking so much time to answer here. I definitely wanted to work on that but I was under the impression that although we now had an agreement to apply PGDLLIMPORT globally a patch itself wasn't really rushed, so I spent the last two days trying to setup a new Windows environment as

  1   2   >