Re: [PATCH] Add tests for Bitmapset

2025-09-22 Thread Michael Paquier
On Fri, Sep 19, 2025 at 02:48:43PM -0400, Greg Burd wrote: > I think I've incorporated your feedback which did indeed make the patch > more readable/crisp and maintainable, thank you. > > coverage:HEADv7v8 > lines..: 87.1 90.5 92.2 bitmap_match() had some duplicated tests. bms

Re: Meson0.57.2 setup failed with option -Duuid=ossp

2025-09-21 Thread Michael Paquier
On Fri, Sep 19, 2025 at 01:23:48PM +0300, Nazir Bilal Yavuz wrote: > Nice catch! I am able to reproduce the problem and I confirm that the > patch fixes it. Right, will fix and backpatch. Please note f039c2244110, that has changed the minimum version to be 0.57.2 on HEAD, and we still allow an ol

Re: GNU/Hurd portability patches

2025-09-21 Thread Michael Paquier
On Sun, Sep 21, 2025 at 09:00:00AM +0300, Alexander Lakhin wrote: > So it seems to me that Hurd is not mature enough yet to test Postgres. I'd say that it is likely not mature enough for production. In terms of testing, that seems kind of OK. However, failures like the one you are reporting here

Re: BF mamba failure

2025-09-20 Thread Michael Paquier
On Tue, Sep 09, 2025 at 04:07:45PM +0300, Kouber Saparev wrote: > Yet again one of our replicas died. Should I file a bug report or > something, what should we do in order to prevent it? Restart the database > every month/week or so?... I don't think we need another bug to report the same problem.

Re: PgStat_HashKey padding issue when passed by reference

2025-09-19 Thread Michael Paquier
On Fri, Sep 19, 2025 at 08:52:24PM -0500, Sami Imseih wrote: > Thank you! I update the CF entry [0] as committed. > > [0] https://commitfest.postgresql.org/patch/6033/ Oops, missed this part. Thanks. -- Michael signature.asc Description: PGP signature

Re: [PATCH] Add tests for Bitmapset

2025-09-18 Thread Michael Paquier
On Thu, Sep 18, 2025 at 02:50:45PM -0400, Greg Burd wrote: > Thanks for all the feedback and time spent reviewing this patch. I > switched out the encode/decode functions to use the nodeToString() and > stringToNode() functions and change all the SQL testing function > signatures to TEXT from BYTE

Re: Incorrect logic in XLogNeedsFlush()

2025-09-18 Thread Michael Paquier
On Thu, Sep 18, 2025 at 05:07:00PM +0530, Dilip Kumar wrote: > I think this comment is a side note which is stating that it is > possible that while XLogNeedFlush() is deciding that based on the > current flush position or min recovery point parallely someone might > flush beyond that point. And i

Re: Resetting recovery target parameters in pg_createsubscriber

2025-09-18 Thread Michael Paquier
On Tue, Sep 16, 2025 at 05:27:43PM +0700, Alyona Vinter wrote: >> This patch also means that pg_createsubscriber is written so as the >> contents added to recovery.conf/postgresql.auto.conf by >> setup_recovery() are never reset if there is a failure in-flight. Is >> that OK or should we also have

Re: PgStat_HashKey padding issue when passed by reference

2025-09-18 Thread Michael Paquier
On Thu, Sep 18, 2025 at 11:52:05AM -0500, Sami Imseih wrote: > v2 looks good to me. Not sure if there are edge cases in which > the assert will succeed even with padding (I can't think of any way > that will be possible), so for now what you have seems good enough. Okay, I've applied that on HEAD

Re: [BUG] temporary file usage report with extended protocol and unnamed portals

2025-09-18 Thread Michael Paquier
On Tue, Sep 16, 2025 at 02:13:39PM -0500, Sami Imseih wrote: > Also, the tests should be checking that we are logging "temporary file: " > before the next statement is logged. > > I split up the actual fix and the corrected tests into separate patches. > They can be committed together if there is

Re: Incorrect logic in XLogNeedsFlush()

2025-09-18 Thread Michael Paquier
On Wed, Sep 17, 2025 at 09:23:05PM -0400, Melanie Plageman wrote: > In terms of comments, I think it is best to update the comment above > XLogNeedsFlush(). Something like : > > /* > - * Test whether XLOG data has been flushed up to (at least) the given > position. > + * Test whether XLOG data h

Re: [BUG] temporary file usage report with extended protocol and unnamed portals

2025-09-17 Thread Michael Paquier
On Wed, Sep 17, 2025 at 10:52:31AM -0500, Sami Imseih wrote: > > One argument for keeping the tests would be that they nicely bring together > > the side-effect (logging) and these use-cases, whereas in the code they're > > pretty distant, making the connection not obvious. > > Another argument

Re: BF mamba failure

2025-09-17 Thread Michael Paquier
On Tue, Sep 16, 2025 at 02:45:03PM +0300, Kouber Saparev wrote: > На пт, 12.09.2025 г. в 3:37 Michael Paquier написа: >> With which process has this cascading standby been created? >> Does the workload of the primary involve a high consumption of OIDs >> for relations, say

Re: Orphan page in _bt_split

2025-09-17 Thread Michael Paquier
On Tue, Sep 16, 2025 at 10:26:12AM +0900, Michael Paquier wrote: > Saying that, this patch sounds like a good idea, making these code > paths a bit more reliable for VACUUM, without paying the cost of an > allocation. At least for HEAD, that's nice to have. Peter, what do > yo

Re: PgStat_HashKey padding issue when passed by reference

2025-09-17 Thread Michael Paquier
ot loved. :D Addressing your points, attached is an updated patch labelled v2. -- Michael From 86f6080503a5e986e7daf771d5d7970f36cc2fe6 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 18 Sep 2025 09:48:44 +0900 Subject: [PATCH v2] Document no-padding rule for PgStat_HashKey This rem

Re: PgStat_HashKey padding issue when passed by reference

2025-09-17 Thread Michael Paquier
On Tue, Sep 16, 2025 at 02:38:20PM -0500, Sami Imseih wrote: > 7d85d87f4d5c35 added code to clear the padding bytes with memset > in anticipation that the key could be changed in the future, in a way > that padding will be introduced. Yep. The argument raised on this thread with the requirement o

Re: Incorrect logic in XLogNeedsFlush()

2025-09-17 Thread Michael Paquier
On Tue, Sep 16, 2025 at 09:40:50AM +0900, Michael Paquier wrote: > As a whole, the patch looks like a good balance, able to satisfy the > new case you want to handle, Melanie. I am guessing that you'd want > to tweak it and apply it yourself, so please feel free. Hearing nothin

Re: Incorrect logic in XLogNeedsFlush()

2025-09-17 Thread Michael Paquier
On Wed, Sep 17, 2025 at 07:05:32PM +0530, Dilip Kumar wrote: > In a recovery scenario, the ControlFile->minRecoveryPoint on a standby > server is continuously updated. This ensures that even in the event of > a crash, a valid recovery point is available. However, if a crashed > standalone server is

Re: Remove PointerIsValid()

2025-09-17 Thread Michael Paquier
On Wed, Sep 17, 2025 at 11:21:10AM -0500, Nathan Bossart wrote: > On Wed, Sep 17, 2025 at 10:15:10AM -0400, Peter Geoghegan wrote: >>> There were the usual concerns about code churn and backpatching and so >>> on, but I think in the end the change is not that big and it's in pretty >>> boring posit

Re: [PATCH] Add tests for Bitmapset

2025-09-17 Thread Michael Paquier
On Wed, Sep 17, 2025 at 10:53:26AM -0400, Greg Burd wrote: > I've updated the hash value for 32bit systems and changed the > bms_values() function to use outBitmapset() via bmsToString(). Oh, right, I also forgot that we have node functions for that. And actually, could it be better to use nodeRe

Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible

2025-09-17 Thread Michael Paquier
On Mon, Sep 08, 2025 at 02:20:33PM -0500, Nathan Bossart wrote: > Committed, thanks for reviewing! Thanks! -- Michael signature.asc Description: PGP signature

Re: [PATCH] Add tests for Bitmapset

2025-09-17 Thread Michael Paquier
On Tue, Sep 16, 2025 at 03:04:39PM -0400, Greg Burd wrote: > I've re-written the set of tests as suggested (I hope!). I've not > re-run the coverage report (yet) to ensure we're at least as well > covered as v3, but attached is v4 for your inspection (amusement?). > I've tried to author the SQL t

Re: question about pending updates in pgstat_report_inj

2025-09-16 Thread Michael Paquier
On Tue, Sep 16, 2025 at 02:19:05PM -0500, Sami Imseih wrote: > On Mon, Sep 15, 2025 at 05:33:45PM -0500, Sami Imseih wrote: >> How about renaming "statent" to "pending" in pgstat_report_inj(), as >> well? That would be a bit more consistent with the subscription stat >> case, at least. > > 0001 L

Re: [PATCH] Add tests for Bitmapset

2025-09-16 Thread Michael Paquier
On Tue, Sep 16, 2025 at 03:00:40PM -0700, Masahiko Sawada wrote: > Thank you for updating the patch. It seems cfbot caught a regression > test error[1] in a 32-bit build. - bitmap_hash [1,3,5] | 49870778 + bitmap_hash [1,3,5] | 1509752520 This one is able the hash value computation being not po

Re: AioCtl Shared Memory size fix

2025-09-15 Thread Michael Paquier
On Mon, Sep 15, 2025 at 02:06:03PM +0200, Matthias van de Meent wrote: > Presumably this was `PgAioHandle io_handles[]` at some point, but now > that it's a pointer it's a proper part of the struct's own size, and > should be treated as such for memory accounting. I would bet on a FLEXIBLE_ARRAY_M

Re: relfilenode statistics

2025-09-15 Thread Michael Paquier
On Thu, Mar 13, 2025 at 02:00:52PM +0500, Kirill Reshke wrote: > Hmm. While it is true that catalog lookups cannot be performed during > crash recovery, is it really necessary to save and retrieve statistics > after a crash? Yes, losing stats on crash is a *very* annoying thing. Having no stats f

Re: Incorrect logic in XLogNeedsFlush()

2025-09-15 Thread Michael Paquier
On Mon, Sep 15, 2025 at 03:02:36PM -0400, Melanie Plageman wrote: > As such, we should clarify the comment above the assert in your patch > to make it clear that the point of the assert is not to check the > logic in XLogFlush() but to protect against drift between > XLogNeedsFlush() and XLogFlush(

Re: [BUG] temporary file usage report with extended protocol and unnamed portals

2025-09-15 Thread Michael Paquier
On Thu, Sep 11, 2025 at 10:28:50AM -0500, Sami Imseih wrote: > > Only question is if we should avoid the extra portal hashtable lookup as > > well, or leave that for a separate patch. > > I prefer a separate thread for this, as it's an optimization of the > existing behavior. - por

Re: [PATCH] Add tests for Bitmapset

2025-09-15 Thread Michael Paquier
On Mon, Sep 15, 2025 at 03:56:30PM -0400, Greg Burd wrote: > Reworked as indicated, thanks for pointing out the anti-pattern I'd > missed. Hmm. Should we try to get something closer in shape to what we do for the SLRU tests, with one SQL function mapping to each C function we are testing? This c

Re: PgStat_HashKey padding issue when passed by reference

2025-09-15 Thread Michael Paquier
On Mon, Sep 15, 2025 at 04:47:27PM -0500, Sami Imseih wrote: > Just to confirm, you are saying we are unlikely to ever add a new field > to the key. Is that correct? I would rather avoid that, yes. -- Michael signature.asc Description: PGP signature

Re: question about pending updates in pgstat_report_inj

2025-09-15 Thread Michael Paquier
ion stat case, at least. -- Michael From 3c0740039c07d2e7bd4ad101ea15f4630a0c4efa Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 16 Sep 2025 11:28:18 +0900 Subject: [PATCH] injection_points: Fix incrementation of variable-numbered stats The pending entry was not used when incrementing its

Re: Resetting recovery target parameters in pg_createsubscriber

2025-09-15 Thread Michael Paquier
On Mon, Sep 15, 2025 at 10:29:47AM +0300, Alexander Korotkov wrote: > I went though this patches. > 1) I've removed the array of parameters. I see it was proposed by > Michael upthread. But I think his proposal came from the fact we walk > trough the parameters twice. But we end up walking troug

Re: Orphan page in _bt_split

2025-09-15 Thread Michael Paquier
On Sun, Sep 14, 2025 at 04:40:54PM +0300, Konstantin Knizhnik wrote: > On 13/09/2025 10:10 PM, Peter Geoghegan wrote: >> I think that _bt_split could easily be switched over to using 2 local >> PGAlignedBlock variables, removing its use of PageGetTempPage. I don't >> think that it is necessary to c

Add support for entry counting in pgstats

2025-09-12 Thread Michael Paquier
test. Thoughts are welcome. -- Michael From 0c924a72f586c385f6ab1a22174b9c3b1cf2dd08 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 12 Sep 2025 16:09:51 +0900 Subject: [PATCH 1/2] Add support for entry counting in pgstats Stats kinds can set an option call track_counts, that will make pg

Re: Incorrect logic in XLogNeedsFlush()

2025-09-11 Thread Michael Paquier
On Fri, Sep 12, 2025 at 10:21:27AM +0530, Dilip Kumar wrote: > Yeah, asserting it at the end makes sense, as we can ensure that > XLogFlush() and XLogNeedsFlush() agree on the same thing without > adding additional overhead. Here is the revised one. Melanie and Jeff, what do you think? -- Michael

Re: Incorrect logic in XLogNeedsFlush()

2025-09-11 Thread Michael Paquier
On Fri, Sep 12, 2025 at 08:45:36AM +0530, Dilip Kumar wrote: > On Fri, Sep 12, 2025 at 8:07 AM Dilip Kumar wrote: >> +1, it really makes XLogFlush() to directly check using >> XLogNeedsFlush() after adding the "WAL inserts are allowed" check in >> XLogNeedsFlush(), this is the best way to avoid an

Re: [PATCH] Add tests for Bitmapset

2025-09-11 Thread Michael Paquier
On Thu, Sep 11, 2025 at 06:56:07AM -0400, Greg Burd wrote: > Just for reference I started this not to increase coverage, which is a good > goal just not the one I had. I was reviewing the API and considering some > changes based on other work I've done. Now that I see how deeply baked in > this c

Re: PgStat_HashKey padding issue when passed by reference

2025-09-11 Thread Michael Paquier
On Thu, Sep 11, 2025 at 10:21:45AM -0500, Sami Imseih wrote: > I don't see how this improves the situation, but will just make it more > difficult to add a new field that requires padding in the future. > > If we are documenting either way, I rather that we just document the need > to pass a key b

Re: BF mamba failure

2025-09-11 Thread Michael Paquier
On Thu, Sep 11, 2025 at 04:35:01PM +0300, Kouber Saparev wrote: > The pattern is the same, although I am not 100% sure that the same replica > is having this - it is a cascaded streaming replica, i.e. a replica of > another replica. Once we had this in October 2024, with version 15.4, then > in Aug

Re: Fix inconsistencies with code and beautify xlog structures description and fin hash_xlog.h

2025-09-11 Thread Michael Paquier
On Wed, Sep 10, 2025 at 05:36:48PM +0500, Andrey Borodin wrote: > Proposed change is correct and in line with a description of > e.g. XLOG_HASH_MOVE_PAGE_CONTENTS. > I'd note that block is for lock only and record does not include > image, but other such cases are not reported in comments. Right,

Re: VM corruption on standby

2025-09-10 Thread Michael Paquier
On Thu, Sep 11, 2025 at 10:59:19AM +1200, Thomas Munro wrote: > FWIW I'm working on a patch set that kills all backends without > releasing any locks when the postmaster exists. Then CVs and other > latch-based stuff should be safe in this context. Work was > interrupted by a vacation but I hope

Re: Stale comment in guc.h; publish listing of setting sources?

2025-09-10 Thread Michael Paquier
On Wed, Sep 10, 2025 at 01:02:46PM -0700, David G. Johnston wrote: > * NB: see GucSource_Names in guc.c if you change this. > > The constant was moved to guc_tables.c I would just remove the file reference. Knowing that GucSource_Names matters is enough to grep for it. Documenting its location

Re: PgStat_HashKey padding issue when passed by reference

2025-09-10 Thread Michael Paquier
ence. So I would just do like attached, documenting at the top of PgStat_HashKey that we should not have padding in it, removing three memset(0) calls that expected it. -- Michael From 104ab93d761746366d6e71e84472f3a3264e7a5b Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 11 Sep 20

Re: someone else to do the list of acknowledgments

2025-09-10 Thread Michael Paquier
On Wed, Sep 10, 2025 at 07:57:26PM +0200, Alvaro Herrera wrote: > Per > https://www.postgresql.org/message-id/tencent_CA843A8385CB3130B9ABC1E55023FC4E4D05%40qq.com > we can credit 清浅 as "Chengwen Wu". Good catch, thanks! I didn't notice his name as this message seems to have been cut from the or

Re: Stale comment in guc.h; publish listing of setting sources?

2025-09-10 Thread Michael Paquier
On Thu, Sep 11, 2025 at 08:48:47AM +0900, Michael Paquier wrote: > I would just remove the file reference. Knowing that GucSource_Names > matters is enough to grep for it. Documenting its location does not > matter. Actually, it matters, as GucSource_Names and GucSource need to be kep

Re: Display is_prev_bucket_same_wrt of xl_hash_squeeze_page

2025-09-10 Thread Michael Paquier
On Wed, Sep 10, 2025 at 06:28:10PM +0500, Kirill Reshke wrote: > While doing some xlog-related task at my job I noticed that pg_waldump > utility does not display `is_prev_bucket_same_wrt` of > xl_hash_squeeze_page record. I had to patch pg_waldump to fix the > issue, because I use pg_waldump outpu

Re: Only one version can be installed when using extension_control_path

2025-09-10 Thread Michael Paquier
On Mon, Sep 08, 2025 at 11:35:42AM -0400, Tom Lane wrote: > Between this and previously-identified problems (commits 81eaaa2c4, > f777d7738), it seems clear that extension_control_path (which is a new > thing in v18) was very poorly thought out. I wonder if it's too late > to revert it so that we

Re: Incorrect logic in XLogNeedsFlush()

2025-09-10 Thread Michael Paquier
On Wed, Sep 10, 2025 at 09:58:08AM -0700, Jeff Davis wrote: > It seems like XLogFlush() and XLogNeedsFlush() should use the same > test, otherwise you could always get some confusing inconsistency. > Right? Even if the checks are duplicated (dependency could be documented as well), it would make s

Re: Incorrect logic in XLogNeedsFlush()

2025-09-10 Thread Michael Paquier
On Wed, Sep 10, 2025 at 11:12:37AM -0400, Melanie Plageman wrote: > On Wed, Sep 10, 2025 at 3:58 AM Michael Paquier wrote: >> You have a good point here, especially knowing that for >> CHECKPOINT_END_OF_RECOVERY case in CreateCheckPoint(), called by the >> checkpointer, we a

Re: Incorrect logic in XLogNeedsFlush()

2025-09-10 Thread Michael Paquier
On Wed, Sep 10, 2025 at 12:48:21PM +0530, Dilip Kumar wrote: >> So, looking at the code, it seems like most places we examine >> ControlFile->minRecoveryPoint, we condition it on "InArchiveRecovery". >> Is this something we want to do in XLogNeedsFlush() to avoid reading >> from ControlFile->minRec

Re: Remove traces of long in dynahash.c

2025-09-09 Thread Michael Paquier
On Tue, Sep 09, 2025 at 10:28:13AM +0100, Dean Rasheed wrote: > So I think there's no point in adding that cap, or any additional > checks in ExecChooseHashTableSize(). You are right that this hardcoded limit introduced in the previous patch was useless. So I have removed that, and applied the re

Re: Memory leak of SMgrRelation object on standby

2025-09-08 Thread Michael Paquier
On Tue, Sep 09, 2025 at 11:58:51AM +0800, 邱宇航 wrote: >> Oops. When redo XLOG_CHECKPOINT_SHUTDOWN, smgrdestroyall should also be >> called, since the startup may not exit on standby. >> >> The patch is updated. True that the situation sucks for the startup process, bloating its memory. That's har

Re: Remove traces of long in dynahash.c

2025-09-08 Thread Michael Paquier
think? -- Michael From 5c81cf6b035368ac2dc9c4b20881a9e13eff5360 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 9 Sep 2025 12:40:04 +0900 Subject: [PATCH v3 1/2] Replace callers of dynahash.h's my_log() by equivalent in pg_bitutils.h All these callers use 4-byte signed integers for their variable

Re: PgStat_HashKey padding issue when passed by reference

2025-09-08 Thread Michael Paquier
On Mon, Sep 08, 2025 at 09:20:14PM -0500, Sami Imseih wrote: > The following will fail the assert since padding is needed for the > new Oid member. > > @@ -53,6 +53,7 @@ typedef struct PgStat_HashKey > { > PgStat_Kind kind; /* statistics entry kind */ > Oid

Re: PgStat_HashKey padding issue when passed by reference

2025-09-08 Thread Michael Paquier
On Mon, Sep 08, 2025 at 12:08:12PM -0400, Andres Freund wrote: > On 2025-09-08 10:25:13 +0900, Michael Paquier wrote: >> Another idea would be to make sure that the sizeof() of the structure >> matches with the sum of the sizeof() for the individual fields in it. >> That&#x

Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

2025-09-08 Thread Michael Paquier
On Mon, Sep 08, 2025 at 09:37:39AM -0400, Robert Treat wrote: > Thanks for your work on this. For those who may not be aware, Sami did > implement a version of this in pg_hint_plan{1}, so that is helpful. I think > it may be worth revisiting this in core, but perhaps once we seen this new > impleme

Re: 回复: Fix segfault while accessing half-initialized hash table in pgstat_shmem.c

2025-09-08 Thread Michael Paquier
On Sat, Sep 06, 2025 at 10:01:24AM +0900, Michael Paquier wrote: > A last thing that I was not able to spend much time on is how much it > is possible to mess up with the shared memory state. If it is worse > than I suspected initially, where an OOM in a first session can cause > cra

Re: [PATCH] Update parser README to include parse_jsontable.c

2025-09-07 Thread Michael Paquier
On Sat, Sep 06, 2025 at 10:38:35AM +0900, Michael Paquier wrote: > That looks wrong since de3600452b61. Will fix, thanks for the report! Applied as 43eb2c541941 and friends. -- Michael signature.asc Description: PGP signature

Re: PgStat_HashKey padding issue when passed by reference

2025-09-07 Thread Michael Paquier
On Sat, Sep 06, 2025 at 10:35:58AM +0900, Michael Paquier wrote: > One idea would be, for example, that keys used with simplehash should > never have any padding at all, and we could force a check in the shape > of a static assertion to force a failure when attempting to compile &g

Re: [PATCH] Update parser README to include parse_jsontable.c

2025-09-05 Thread Michael Paquier
On Fri, Sep 05, 2025 at 06:49:42PM +0530, Karthik wrote: > This patch fixes a missing entry in the parser README. The file > parse_jsontable.c exists but wasn't documented in the README. > > The patch adds the missing line with an appropriate description. > > Please find the patch attached. That

Re: PgStat_HashKey padding issue when passed by reference

2025-09-05 Thread Michael Paquier
On Thu, Sep 04, 2025 at 11:50:15AM -0500, Sami Imseih wrote: > Perhaps calling this a compiler bug is not appropriate. > However, memset is in fact called when the key is created > > ``` > /* clear padding */ > memset(&key, 0, sizeof(struct PgStat_HashKey)); > ``` > > but the zeroed out paddi

Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible

2025-09-05 Thread Michael Paquier
On Fri, Sep 05, 2025 at 01:12:49PM -0500, Nathan Bossart wrote: > How does this look? +# We can only test security labels if both the old and new installations +# have dummy_seclabel. +my $test_seclabel = 1; +$old->start; +if (!$old->check_extension('dummy_seclabel')) +{ +

Re: 回复: Fix segfault while accessing half-initialized hash table in pgstat_shmem.c

2025-09-05 Thread Michael Paquier
On Thu, Sep 04, 2025 at 03:49:19PM +0800, Rider wrote: > And, the PG_RE_THROW() within the PG_CATCH block causes a non-local jump, > immediately aborting the current execution path to handle the error at a > higher level. This guarantees that the code following PG_END_TRY is > unreachable in the er

Re: 回复: Fix segfault while accessing half-initialized hash table in pgstat_shmem.c

2025-09-05 Thread Michael Paquier
On Fri, Sep 05, 2025 at 09:46:55PM +0100, Mikhail Kot wrote: > Do you want me to update the patch based on your suggestion on fault > injection, or update the try/catch to the callers as discussed, or > it's good to be included in Postgres? I would prefer the approach of letting the callers deal w

Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible

2025-09-04 Thread Michael Paquier
On Thu, Sep 04, 2025 at 08:23:58PM -0500, Nathan Bossart wrote: > Ah, I'd forgotten about EXTRA_INSTALL. That simplifies things. There's > enough special handling for large objects in pg_upgrade that I think we > ought to test it end-to-end, so I sneaked it into 006_tranfer_modes.pl. > WDYT? Nea

Re: Update outdated references to SLRU ControlLock

2025-09-04 Thread Michael Paquier
On Fri, Sep 05, 2025 at 09:47:24AM +0800, Julien Rouhaud wrote: > I caught a couple of other minor outdated things in the comments when > finishing > rebasing my own work: > > - long_segment_names has been added to SimpleLruInit > - SlruRecentlyUsed is not a macro anymore Right. The source of e

Re: Refactoring: Use soft error reporting for *_opt_error functions

2025-09-04 Thread Michael Paquier
On Fri, Sep 05, 2025 at 08:10:00AM +0900, Michael Paquier wrote: > Sure. I'll handle it. Thanks. Applied after a few tweaks, including changes to the comments, the suggestion of "division_by_zero" for the goto labels, and splitting the patch into two parts for pg_lsn and n

Re: Refactoring: Use soft error reporting for *_opt_error functions

2025-09-04 Thread Michael Paquier
On Thu, Sep 04, 2025 at 10:50:38AM +0100, Dean Rasheed wrote: > I don't mind. I haven't looked at it too closely, but I'm broadly > happy with it. I think any issues are probably now minor cosmetic > things, so if you've been looking in more detail and are happy to > commit it, then go ahead. Other

Re: 回复: Fix segfault while accessing half-initialized hash table in pgstat_shmem.c

2025-09-04 Thread Michael Paquier
On Thu, Sep 04, 2025 at 02:31:34AM +, Steven Niu wrote: > If pgstat_init_entry() errors on OOM, what would it returns to shheader, NULL? > That would bring trouble to dshash_delete_entry(). Based on the proposal of patch 0002, the code would throw an error after cleaning up the shared memory s

Re: Refactoring: Use soft error reporting for *_opt_error functions

2025-09-04 Thread Michael Paquier
On Wed, Sep 03, 2025 at 10:41:14PM +0800, jian he wrote: > would any extensions using these functions (such as > numeric_int4_opt_error) may encounter upgrade compatibility issues in > the future? That would be also the point, so as callers are made aware of these "upgraded" versions. I have foun

Re: Orphan page in _bt_split

2025-09-04 Thread Michael Paquier
On Wed, Sep 03, 2025 at 09:32:41AM +0300, Konstantin Knizhnik wrote: > On 01/09/2025 10:18 PM, Peter Geoghegan wrote: >> Also rethinking this aspect: a checksum failure probably *isn't* going >> to make much difference. Since that'll also cause bigger problems for >> VACUUM than logging one of thes

Re: Fix segfault while accessing half-initialized hash table in pgstat_shmem.c

2025-09-04 Thread Michael Paquier
that none of that seems worth a backpatch, we have an history of treating unlikely-going-to-happen errors like OOMs as HEAD-only improvements. This one is of the same class. -- Michael From 6fe33b6446f1c57ca7570f742f39f844e36a7c23 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 4 Sep 20

Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible

2025-09-03 Thread Michael Paquier
On Tue, Sep 02, 2025 at 09:43:40AM -0500, Nathan Bossart wrote: > Do you think a new pg_upgrade test for security labels is worth the > trouble? It seems doable, but it'd be an awfully expensive test for this. > On the other hand, I'm not sure there's any coverage for pg_upgrade with > security la

Re: Remove traces of long in dynahash.c

2025-09-03 Thread Michael Paquier
On Wed, Sep 03, 2025 at 02:48:40PM +0200, Peter Eisentraut wrote: > Taking a look at your previous patch with the changes from long to int64, I > think there is something that still doesn't fit. > > For example, taking a look at the callers of hash_estimate_size(int64, > Size), they pass either in

Re: 回复: Fix segfault while accessing half-initialized hash table in pgstat_shmem.c

2025-09-03 Thread Michael Paquier
On Wed, Sep 03, 2025 at 07:22:00AM +, Steven Niu wrote: > So unless dsa_allocate() can ensure never returns InvalidDsaPointer, > there is risk of SegV. In fact the function dsa_allocate() does > return InvalidDsaPointer in some cases. > > So, maybe should we add pointer check in all places wh

Re: Orphan page in _bt_split

2025-09-03 Thread Michael Paquier
On Mon, Sep 01, 2025 at 02:35:56PM -0400, Peter Geoghegan wrote: > I remember that when I worked on what became commit 9b42e71376 back in > 2019 (which fixed a similar problem caused by the INCLUDE index > patch), Tom suggested that we do things this way defensively (without > being specifically aw

Re: Refactoring: Use soft error reporting for *_opt_error functions

2025-09-03 Thread Michael Paquier
On Tue, Sep 02, 2025 at 02:41:23PM +0530, Amul Sul wrote: > The updated version is attached. In addition to the *_opt_error() > functions, it also renames pg_lsn_in_internal to pg_lsn_in_safe and > incorporates soft error handling. Looks globally sensible to me. I was wondering for a bit if the J

Re: Fix segfault while accessing half-initialized hash table in pgstat_shmem.c

2025-09-03 Thread Michael Paquier
On Tue, Sep 02, 2025 at 09:09:54PM +0100, Mikhail Kot wrote: > The error originates from pgstat_shmem.c file where shhashent is left in > half-initialized state if pgstat_init_entry(), calling dsa_allocate0(), errors > out with OOM. Then shhashent causes a segmentation fault on access. I propose >

Re: Get rid of pgstat_count_backend_io_op*() functions

2025-09-02 Thread Michael Paquier
On Tue, Sep 02, 2025 at 12:42:54PM -0400, Andres Freund wrote: > On 2025-09-02 08:19:22 +0900, Michael Paquier wrote: >> On Mon, Sep 01, 2025 at 02:07:27PM +, Bertrand Drouvot wrote: >>> Instead, it now copies the IO pending stats to the backend IO pending stats >>&

Re: Get rid of pgstat_count_backend_io_op*() functions

2025-09-02 Thread Michael Paquier
On Mon, Sep 01, 2025 at 02:07:27PM +, Bertrand Drouvot wrote: > Instead, it now copies the IO pending stats to the backend IO pending stats > when > the pending IO stats are flushed. This behaves the same way as for some > relation > and database stats (see pgstat_relation_flush_cb()). > > I

Re: Fix pg_waldump to exit cleanly at end of WAL

2025-09-02 Thread Michael Paquier
On Wed, Sep 03, 2025 at 09:11:15AM +0900, Fujii Masao wrote: > Can pg_waldump really distinguish between the end of WAL and corruption? I don't think you can really do that reliably, as some of the messages marking the end of WAL could also be bumped into upon a corruption, as far as I recall. We

Re: Update outdated references to SLRU ControlLock

2025-09-02 Thread Michael Paquier
On Mon, Sep 01, 2025 at 01:19:41PM +0800, Julien Rouhaud wrote: > I don't really have a preference. Bank lock is shorter but may be a bit more > obscure, especially outside slru.c, so using "SLRU bank lock" could be better > indeed. Okay, I have used "SLRU bank lock" and backpatched that down to

Re: Refactoring: Use soft error reporting for *_opt_error functions

2025-09-02 Thread Michael Paquier
On Tue, Sep 02, 2025 at 12:40:25PM +0530, Amul Sul wrote: > Just a quick question regarding the naming conventions. It looks like > we have a choice between two options for consistency. Should we rename > the pg_lsn_in_internal function by replacing "_internal" with "_safe", > or should we rename a

Re: Refactoring: Use soft error reporting for *_opt_error functions

2025-09-01 Thread Michael Paquier
On Mon, Sep 01, 2025 at 12:21:18PM +0100, Dean Rasheed wrote: > On Mon, 1 Sept 2025 at 10:36, Amul Sul wrote: >> I believe we should update all *_opt_error functions to use the new >> soft error reporting infrastructure instead of boolean flags -- did >> the same in the attached patch. I am not su

Re: Orphan page in _bt_split

2025-09-01 Thread Michael Paquier
On Mon, Sep 01, 2025 at 03:04:58PM -0400, Peter Geoghegan wrote: > This hazard has existing since commit 8fa30f906b, from 2010. That's > the commit that introduced the general idea of making _bt_split zero > its rightpage in order to make it safe to throw an ERROR instead of just > PANICing. Thank

Re: Replace magic numbers with strategy numbers for B-tree indexes

2025-09-01 Thread Michael Paquier
On Mon, Sep 01, 2025 at 09:04:04PM +0700, Daniil Davydov wrote: > I don't think that we can just create different enums for each index > strategies. > We have (for example) ScanKey functionality, which can work with different > indexes (and such a functions has a uint16 argument for strategy numbe

Re: Serverside SNI support in libpq

2025-08-31 Thread Michael Paquier
On Wed, Aug 27, 2025 at 09:49:34PM +0200, Daniel Gustafsson wrote: > When looking into why the SNI tests failed on Windows I think I found a > pre-existing issue that we didn't have tests for, which my patch added tests > for and thus broke. > > The test I added was to check restarting and reloadi

Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible

2025-08-31 Thread Michael Paquier
On Thu, Aug 14, 2025 at 10:22:02AM -0500, Nathan Bossart wrote: > Here is a patch. For background, the reason this is limited to upgrades > from v16 and newer is because the aclitem data type (needed by > pg_largeobject_metadata.lomacl) changed its storage format in v16 (see > commit 7b378237aa).

Re: Orphan page in _bt_split

2025-08-31 Thread Michael Paquier
On Mon, Sep 01, 2025 at 08:03:18AM +0300, Konstantin Knizhnik wrote: > _bp_getbuf just reads page in a shared buffer. But ReadBuffer actually can > invoke half of Postgres code (locating buffer, evicting victim, writing to > SMGR,..., So there are a lot of places where error can be thrown (includin

Re: Update outdated references to SLRU ControlLock

2025-08-31 Thread Michael Paquier
On Mon, Sep 01, 2025 at 11:32:41AM +0800, Julien Rouhaud wrote: > Note that the main comment of slru.c still has one paragraph that mentions > "bank control lock" consistently before switching to just "control lock" in > the > next paragraph. I'm assuming that it's ok in that context as it seems

Re: Remove traces of long in dynahash.c

2025-08-31 Thread Michael Paquier
on of pg_ceil_log2_32_bound() and pg_ceil_log2_64_bound(). At the end, next_pow2_int64() and next_pow2_int() are a lesser deal to me, being static to dynahash.c. With that in mind, I am finishing with the attached. Less ambitious, still it's a nice cleanup IMO. What do you think? -- Michael

Re: Resetting recovery target parameters in pg_createsubscriber

2025-08-31 Thread Michael Paquier
On Mon, Sep 01, 2025 at 02:06:34AM +, Hayato Kuroda (Fujitsu) wrote: > WriteRecoveryConfig() has been used to setup the recovery parameters. Can we > follow the way to restore them? > > Also, can we add a test to 040_pg_createsubscriber? IIUC it is enough to check > one of recovery parameter i

Re: Buffer locking is special (hints, checksums, AIO writes)

2025-08-31 Thread Michael Paquier
On Wed, Aug 27, 2025 at 10:03:08AM -0400, Robert Haas wrote: > If we were to use the existing PostgreSQL naming convention, I think > I'd probably argue that the nearest parallel to this level is > ShareUpdateExclusive: a self-exclusive lock level that permits > ordinary table access to continue wh

Re: Generate pgstat_count_slru*() functions for slru using macros

2025-08-31 Thread Michael Paquier
On Fri, Aug 29, 2025 at 03:02:23PM +, Bertrand Drouvot wrote: > I was looking at pgstat_slru.c (I've in mind to provide some of those metrics > per backend), and realized that the same code pattern is repeated 7 times. So, the gist of the change is centralized around get_slru_entry(), renaming

Re: hash + LRC better than CRC?

2025-08-31 Thread Michael Paquier
On Sun, Aug 31, 2025 at 08:43:20PM +0700, John Naylor wrote: > 1) It's trivial to reduce a 64-bit universal hash to a 32 (or > whatever)-bit universal hash by composing it with another universal > hash function -- a convenient example is Dietzfelbinger's > multiplicative hash (see section 2.3 in [1

Re: Adding REPACK [concurrently]

2025-08-31 Thread Michael Paquier
On Wed, Aug 27, 2025 at 10:22:24AM +0200, Mihail Nikalayeu wrote: > That worries me - it is not the behaviour someone expects from a > database by default. At least the warning should be much more visible > and obvious. > I think most of user will expect the same guarantees as [CREATE|RE] > INDEX C

Re: Invalid remote sampling test in postgres_fdw.

2025-08-31 Thread Michael Paquier
On Tue, Aug 12, 2025 at 08:58:47AM +0900, Michael Paquier wrote: > I would suggest waiting a bit for the resolution of the other bug, and > make sure that we have at least one reference to the trick with the DO > block. While the way you are writing the test is more readable, > th

Re: Per backend relation statistics tracking

2025-08-25 Thread Michael Paquier
On Mon, Aug 25, 2025 at 05:51:38PM -0500, Sami Imseih wrote: > I have not gone through them in detail yet, but +1 on adding backend activity > stats. This provides another level of drill down to spot anomalous sessions or > different patterns across applications. I also think we will want more than

Re: Remove redundant assignment of a variable in function AlterPublicationTables

2025-08-21 Thread Michael Paquier
On Fri, Aug 22, 2025 at 09:55:23AM +1000, Peter Smith wrote: > To summarise: > Only ~4 places are redundantly assigning isNull values before calling > functions that use &isNull. > But ~400 function calls that are passing &isNull do not pre-assignment > of that variable. > > Choosing to keep this

Re: Remove unneeded cast in heap_xlog_lock.

2025-08-21 Thread Michael Paquier
On Fri, Aug 22, 2025 at 10:41:15AM +0900, Richard Guo wrote: > Although these casts are unnecessary for sure, I'm not sure if it's > worth making the code changes to fix them. That's sort of the point. This is not code that needs to be fixed, because it's not broken. -- Michael signature.asc De

  1   2   3   4   5   6   7   8   9   10   >