Hi, my previous review posts did not cover the test code.
Here are my review comments for the v44-0001 test code
==
TEST CASE #1
1.
+# Wait for the inactive replication slot to be invalidated.
+$standby1->poll_query_until(
+ 'postgres', qq[
+ SELECT COUNT(slot_name) = 1 FROM pg_replication_s
At Mon, 2 Sep 2024 14:55:52 +, Bertrand Drouvot
wrote in
> Hi hackers,
>
> Please find attached a patch to implement $SUBJECT.
>
> While pg_stat_io provides cluster-wide I/O statistics, this patch adds a new
> pg_my_stat_io view to display "my" backend I/O statistics and a new
> pg_stat_ge
On Tue, Sep 03, 2024 at 12:00:19PM +1000, Peter Smith wrote:
> Here is the rebased patch set v10*. Everything is the same as before
> except now there are only 7 patches instead of 8. The previous v9-0001
> ("bool") patch no longer exists because those changes are now already
> present in HEAD.
>
Hi,
On Tue, Sep 03, 2024 at 10:43:14AM +0530, Amit Kapila wrote:
> On Mon, Sep 2, 2024 at 9:14 AM shveta malik wrote:
> >
> > On Mon, Sep 2, 2024 at 5:47 AM Peter Smith wrote:
> > >
> > >
> > > To summarize, the current description wrongly describes the field as a
> > > time duration:
> > >
On Tue, Sep 03, 2024 at 04:35:28AM +, Bertrand Drouvot wrote:
> I understand why pg_xlog and pg_wal both need to be listed here, but I don't
> get why the proposed changes were "wrong". Or, are you saying that if for any
> reason PG_TBLSPC_DIR needs to be changed that would not work
> anymore?
On 18/7/2024 14:49, Matthias van de Meent wrote:
Aside: Arguably, checking for commutator operators would not be
incorrect when looking at it from "applied operators" point of view,
but if that commutative operator isn't registered as opposite ordering
of the same btree opclass, then we'd probabl
On Tue, Sep 3, 2024 at 5:00 PM Alexander Lakhin wrote:
> From a bird's eye view, new v17-vs-v16 comparison has only 87 "worse",
> while the previous one had 115 (it requires deeper analysis, of course, but
> still...).
Any chance you could share that whole pgdata dir with me, assuming it
compres
On 28.08.24 12:04, Aleksander Alekseev wrote:
Hi,
On 04.10.23 16:37, Peter Eisentraut wrote:
On 03.10.23 13:28, Aleksander Alekseev wrote:
While examining the code for similar places I noticed that the
following functions can also be const'ified:
- XLogRegisterData (?)
I don't think this
On Tue, Sep 3, 2024 at 10:43 AM Amit Kapila wrote:
>
> > >
> > > To summarize, the current description wrongly describes the field as a
> > > time duration:
> > > "The time since the slot has become inactive."
> > >
> > > I suggest replacing it with:
> > > "The slot has been inactive since this ti
On Tue, Sep 03, 2024 at 05:00:54AM +, Bertrand Drouvot wrote:
> Should we add a few words about this new callback (and the others in
> PgStat_KindInfo while at it) in the "Custom Cumulative Statistics" section of
> xfunc.sgml?
Not sure if it is worth having. This adds extra maintenance and
th
On Tue, Sep 03, 2024 at 02:24:32PM +0900, Michael Paquier wrote:
> On Mon, Sep 02, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote:
> > I've gathered another bunch of defects with the possible substitutions.
> > Please take a look:
> > pgstat_add_kind -> pgstat_register_kind (see 7949d9594)
>
> A
At Mon, 2 Sep 2024 15:44:45 +0200, Daniel Gustafsson wrote in
> Summarizing the thread it seems consensus is using XLOG_CONTROL_FILE
> consistently as per the original patch.
>
> A few comments on the patch though:
>
> - * reads the data from $PGDATA/global/pg_control
> + * reads the data from
On Mon, Sep 02, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote:
> I've gathered another bunch of defects with the possible substitutions.
> Please take a look:
> pgstat_add_kind -> pgstat_register_kind (see 7949d9594)
And here I thought I took care of these inconsistencies. This one is
on me so
On Mon, Sep 2, 2024 at 9:14 AM shveta malik wrote:
>
> On Mon, Sep 2, 2024 at 5:47 AM Peter Smith wrote:
> >
> >
> > To summarize, the current description wrongly describes the field as a
> > time duration:
> > "The time since the slot has become inactive."
> >
> > I suggest replacing it wit
Hi,
On Tue, Sep 03, 2024 at 10:52:20AM +0900, Michael Paquier wrote:
> Hi all,
>
> Currently, the backend-level initialization of pgstats happens in
> pgstat_initialize(), where we are using a shortcut for the WAL stats,
> with pgstat_init_wal().
>
> I'd like to propose to add a callback for tha
Hello Thomas,
02.08.2024 12:00, Alexander Lakhin wrote:
I had payed attention to:
Best pg-src-17--.* worse than pg-src-16--.* by 57.9 percents (225.11 > 142.52):
pg_tpcds.query15
Average pg-src-17--.* worse than pg-src-16--.* by 55.5 percents (230.57 >
148.29): pg_tpcds.query15
in May, p
On Thu, Aug 29, 2024 at 9:35 PM jian he wrote:
>
> On Thu, Aug 29, 2024 at 8:15 PM Peter Eisentraut wrote:
> >
> >
> > The new patch does some rebasing and contains various fixes to the
> > issues you presented. As I mentioned, I'll look into improving the
> > rewriting.
>
>
> based on your la
Hi all,
The last TODO item I had in my bucket about the generalization of
pgstats is the option to a better control on the flush of the stats
depending on the kind for fixed-numbered stats. Currently, this is
controlled by pgstat_report_stat(), that includes special handling for
WAL, IO and SLRU
Hi,
On Tue, Sep 03, 2024 at 09:15:50AM +0900, Michael Paquier wrote:
> On Fri, Aug 30, 2024 at 12:21:29PM +, Bertrand Drouvot wrote:
> > That said, I don't have a strong opinion on this one, I think that also
> > makes
> > sense to leave it as it is. Please find attached v4 doing so.
>
> The
On Fri, 5 Jul 2024 at 18:38, Heikki Linnakangas wrote:
>
> On 05/07/2024 14:07, vignesh C wrote:
> > On Thu, 4 Jul 2024 at 16:52, Heikki Linnakangas wrote:
> >>
> >> I'm don't quite understand the problem we're trying to fix:
> >>
> >>> Currently the launcher's latch is used for the following: a)
Richard Guo writes:
> Attached is a WIP patch that marks PHVs that need to be kept because
> they are serving to isolate subexpressions, and removes all other PHVs
> in remove_nulling_relids_mutator if their phnullingrels bits become
> empty. One problem with it is that a PHV's contained expressi
In [1] there was a short discussion about removing no-op
PlaceHolderVars. This thread is for continuing that discussion.
We may insert PlaceHolderVars when pulling up a subquery that is
within the nullable side of an outer join: if subquery pullup needs to
replace a subquery-referencing Var that
Jehan-Guillaume de Rorthais 于2024年9月3日周二 05:02写道：
> Hi,
>
> On Tue, 20 Aug 2024 23:09:27 -0400
> Alvaro Herrera wrote:
>
> > On 2024-Aug-20, Jehan-Guillaume de Rorthais wrote:
> >
> > > I'm back on this issue as well. I start poking at this patch to review
> it,
> > > test it, challenge it and t
Hi.
The cfbot was reporting my patches needed to be rebased.
Here is the rebased patch set v10*. Everything is the same as before
except now there are only 7 patches instead of 8. The previous v9-0001
("bool") patch no longer exists because those changes are now already
present in HEAD.
I hope t
Hi all,
Currently, the backend-level initialization of pgstats happens in
pgstat_initialize(), where we are using a shortcut for the WAL stats,
with pgstat_init_wal().
I'd like to propose to add a callback for that, so as in-core or
custom pgstats kinds can assign custom actions when initializing
On Mon, Aug 19, 2024 at 1:35 AM Peter Eisentraut wrote:
> On 17.08.24 00:01, Thomas Munro wrote:
> > I think that's fine. I don't really like the word "prefetch", could
> > mean many different things. What about "requires OS support for
> > issuing read-ahead advice", which uses a word that appe
On Tue, Sep 03, 2024 at 09:22:58AM +0900, Michael Paquier wrote:
> On Thu, Aug 29, 2024 at 09:49:44AM -0500, Nathan Bossart wrote:
>> IMHO continuing to allow partitioned tables to be marked UNLOGGED just
>> preserves the illusion that it does something. An ERROR could help dispel
>> that misconce
On Mon, Sep 02, 2024 at 05:08:03PM +0300, Heikki Linnakangas wrote:
> Hmm, I'm a bit disappointed this doesn't address replication. It makes sense
> that scans are counted separately on a standby, but it would be nice if
> stats like last_vacuum were propagated from primary to standbys. I guess
>
On Thu, Aug 29, 2024 at 09:49:44AM -0500, Nathan Bossart wrote:
> IMHO continuing to allow partitioned tables to be marked UNLOGGED just
> preserves the illusion that it does something. An ERROR could help dispel
> that misconception.
Okay. This is going to be disruptive if we do nothing about p
On Fri, Aug 30, 2024 at 12:21:29PM +, Bertrand Drouvot wrote:
> That said, I don't have a strong opinion on this one, I think that also makes
> sense to leave it as it is. Please find attached v4 doing so.
The changes in astreamer_file.c are actually wrong regarding the fact
that should_allow_
On Mon, Sep 02, 2024 at 03:00:32PM +, Bertrand Drouvot wrote:
> On Fri, Aug 23, 2024 at 07:32:16AM +, Bertrand Drouvot wrote:
>> FWIW, I think that would be great and plan to have a look at this (unless
>> someone
>> beats me to it).
>
> FWIW, here is the patch proposal for per backend I/
On 2024-09-01 Su 2:46 PM, sia kc wrote:
Hello everyone
I am a developer interested in this project. Had a little involvement
with MariaDB and now I like to work on Postgres. Never worked with
mailing lists so I am not sure if this is the way I should interact.
Liked to be pointed to some tas
Hi,
On Tue, 20 Aug 2024 23:09:27 -0400
Alvaro Herrera wrote:
> On 2024-Aug-20, Jehan-Guillaume de Rorthais wrote:
>
> > I'm back on this issue as well. I start poking at this patch to review it,
> > test it, challenge it and then report here.
> >
> > I'll try to check if some other issues migh
Peter Eisentraut writes:
> I think we can apply these patches now to check this off the list of
> not-thread-safe functions to check.
+1 for the first patch. I'm less happy with
- static char errbuf[36];
+ static char errbuf[128];
As a minor point, shouldn't this be
+ stati
There are only a few (not necessarily thread-safe) strerror() calls in
the backend; most other potential users use %m in a format string.
In two cases, the reason for using strerror() was that we needed to
print the error message twice, and so errno has to be reset for the
second time. And/or
On 14/8/2024 23:05, Imseih (AWS), Sami wrote:
I think the testing discussion should be moved to a different thread.
What do you think?
See v4.
0001 deals with reporting queryId in exec_execute_message and
exec_bind_message.
0002 deals with reporting queryId after a cache invalidation.
There
On 9/2/24 9:06 PM, Andreas Karlsson wrote:
On 9/2/24 4:23 AM, Xing Guo wrote:
Thanks for testing it! I spotted another unnecessary store instruction
and added it in my V2 patch.
Another well-spotted unnecessary store. Nice!
I think this patch is ready for committer. It is simple and pretty
o
On 9/2/24 4:23 AM, Xing Guo wrote:
Thanks for testing it! I spotted another unnecessary store instruction
and added it in my V2 patch.
Another well-spotted unnecessary store. Nice!
I think this patch is ready for committer. It is simple and pretty
obviously correct.
Andreas
Thanks for the contribution.
I took a look at the patch, and it works as advertised. It's too late
for the September commitfest, but I took the liberty of registering
your patch for the November CF [1]. In the course of that, I found an
older thread proposing this feature seven years ago [2]. That
Hello,
12.08.2024 14:59, David Rowley wrote:
(I know Daniel mentioned he'd get to these, but the ScanDirection one
was my fault and I needed to clear that off my mind. I did a few
others while on this topic.)
Thank you, David, for working on that!
I've gathered another bunch of defects with t
On 9/2/24 01:53, Robert Haas wrote:
> On Sun, Sep 1, 2024 at 3:30 PM Tomas Vondra wrote:
>> I don't think that's possible with hard-coded size of the array - that
>> allocates the memory for everyone. We'd need to make it variable-length,
>> and while doing those benchmarks I think we actually alr
On Tue, Aug 20, 2024 at 4:10 PM Amit Kapila wrote:
>
> On Thu, Aug 15, 2024 at 9:31 PM vignesh C wrote:
> > Since we are applying invalidations to all in-progress transactions,
> > the publisher will only replicate half of the transaction data up to
> > the point of invalidation, while the remain
Hi,
On Fri, 30 Aug 2024 at 21:36, Jacob Champion
wrote:
>
> On Wed, Aug 28, 2024 at 8:21 AM Nazir Bilal Yavuz wrote:
> > I do not exactly remember the reason but I think I copied the same
> > behavior as before, PG_TEST_EXTRA variable was checked in the
> > src/test/Makefile so I exported it the
Hi,
On Fri, Aug 23, 2024 at 07:32:16AM +, Bertrand Drouvot wrote:
> Hi,
>
> On Wed, Mar 08, 2023 at 04:34:38PM -0800, Andres Freund wrote:
> > On 2023-03-08 12:55:34 +0100, Drouvot, Bertrand wrote:
> > > - pg_stat_io is "global" across all sessions. So, even if one session is
> > > doing som
Hi hackers,
Please find attached a patch to implement $SUBJECT.
While pg_stat_io provides cluster-wide I/O statistics, this patch adds a new
pg_my_stat_io view to display "my" backend I/O statistics and a new
pg_stat_get_backend_io() function to retrieve the I/O statistics for a given
backend pid
It'd not be such an issue if we updated stats during recovery, but I
think think we're doing that. Perhaps we should, which might also help
on replicas - no idea if it's feasible, though.
Stats on replicas are considered an independent thing AFAIU (scans are
counted for example in read-only quer
On Thu, 2024-08-29 at 19:33 +0200, Pavel Stehule wrote:
> > > > > > + /*
> > > > > > + * Although svar is freshly validated in this point, the
> > > > > svar->is_valid can
> > > > > > + * be false, due possible accepting invalidation message
> > > > > inside domain
> > > > > > +
> On 27 Apr 2024, at 11:12, Peter Eisentraut wrote:
>
> On 26.04.24 22:51, Tom Lane wrote:
>> Robert Haas writes:
>>> On Wed, Apr 24, 2024 at 8:04 PM Michael Paquier wrote:
Not sure that I would bother with a second one. But, well, why not if
people want to rename it, as long as you
Hi,
On Thu, 29 Aug 2024 at 15:16, Peter Eisentraut wrote:
>
> I also committed the two patches that renamed the existing test files,
> so those are not included here anymore.
>
> The new patch does some rebasing and contains various fixes to the
> issues you presented. As I mentioned, I'll look
On Wed, Aug 21, 2024 at 6:52 PM Dean Rasheed wrote:
>
> On Wed, 21 Aug 2024 at 08:00, Peter Eisentraut wrote:
> >
> > On 08.08.24 20:22, Dean Rasheed wrote:
> > > Looking at the rewriter changes, it occurred to me that it could
> > > perhaps be done more simply using ReplaceVarsFromTargetList() f
> On 15 Aug 2024, at 00:42, Jacob Champion
> wrote:
> It's pretty frustrating to hear about a "transition" when there is
> nothing to transition to.
I guess they prefer that orgs transition back to just using CRL's.
> Anyways, I look forward to seeing how broken my crystal ball is this
> time.
On Mon, Sep 2, 2024 at 3:32 PM Amit Kapila wrote:
>
> On Mon, Sep 2, 2024 at 11:21 AM Dilip Kumar wrote:
> >
> > While working on some other code I noticed that in
> > FindReplTupleInLocalRel() there is an assert [1] that seems to be
> > passing IndexRelation to GetRelationIdentityOrPK() whereas
> On 2 Sep 2024, at 10:03, Daniel Gustafsson wrote:
>
>> On 23 Aug 2024, at 01:56, Michael Paquier wrote:
>>
>> On Thu, Aug 22, 2024 at 11:13:15PM +0200, Daniel Gustafsson wrote:
>>> On 22 Aug 2024, at 02:31, Michael Paquier wrote:
Just do it :)
>>>
>>> That's my plan, I wanted to wait a
Hi,
On Sat, 31 Aug 2024 at 02:51, Noah Misch wrote:
>
> To read blocks 10 and 11, I would expect to initialize the struct with one of:
>
> { .first=10, .nblocks=2 }
> { .first=10, .last_inclusive=11 }
> { .first=10, .last_exclusive=12 }
>
> With the patch's API, I would need {.first=10,.nblocks=1
> On 2 Sep 2024, at 13:06, Dagfinn Ilmari Mannsåker wrote:
>
> Greg Sabino Mullane writes:
>
>> I normally wouldn't mention my blog entries here, but this one was about
>> the hackers mailing list, so wanted to let people know about it in case you
>> don't follow Planet Postgres. I scanned the
On Fri, 30 Aug 2024 at 21:43, Matthias van de Meent
wrote:
>
> On Mon, 19 Aug 2024 at 13:43, Matthias van de Meent
> wrote:
> >
> > On Sun, 11 Aug 2024 at 21:44, Peter Geoghegan wrote:
> > >
> > > On Tue, Aug 6, 2024 at 6:31 PM Matthias van de Meent
> > > wrote:
> > > > +1, LGTM.
> > > >
> > >
Greg Sabino Mullane writes:
> I normally wouldn't mention my blog entries here, but this one was about
> the hackers mailing list, so wanted to let people know about it in case you
> don't follow Planet Postgres. I scanned the last year's worth of posts and
> gathered the most used acronyms and j
Alvaro Herrera 于2024年8月31日周六 11:59写道：
> Hello
>
> Here I present another attempt at making not-null constraints be
> catalogued. This is largely based on the code reverted at 9ce04b50e120,
> except that we now have a not-null constraint automatically created for
> every column of a primary key,
On 01/09/2024 09:27, Andres Freund wrote:
The main reason I had previously implemented WAL AIO etc was to know the
design implications - but now that they're somewhat understood, I'm planning
to keep the patchset much smaller, with the goal of making it upstreamable.
+1 on that approach.
To s
On Mon, Sep 2, 2024 at 11:21 AM Dilip Kumar wrote:
>
> While working on some other code I noticed that in
> FindReplTupleInLocalRel() there is an assert [1] that seems to be
> passing IndexRelation to GetRelationIdentityOrPK() whereas it should
> be passing normal relation.
>
Agreed. But this sho
On Sat, Aug 31, 2024 at 1:45 PM Bharath Rupireddy
wrote:
>
> Please find the attached v44 patch with the above changes. I will
> include the 0002 xid_age based invalidation patch later.
>
It is better to get the 0001 reviewed and committed first. We can
discuss about 0002 afterwards as 0001 is in
On Mon, Sep 2, 2024 at 1:28 PM shveta malik wrote:
>
> On Mon, Sep 2, 2024 at 4:20 AM Peter Smith wrote:
> >
> > On Fri, Aug 30, 2024 at 4:24 PM shveta malik wrote:
> > >
> > > On Fri, Aug 30, 2024 at 10:53 AM Peter Smith
> > > wrote:
> > > >
> > ...
> > > > 2. Arrange all the counts into an i
Hi,
While reviewing another thread I was looking at the view
'pg_stats_subscription_stats' view. In particular, I was looking at
the order of the "*_count" columns of that view.
IMO there is an intuitive/natural ordering for the logical replication
operations (LR) being counted. For example, LR "
There's a category[1] of random build farm/CI failures where Windows
behaves differently and our stuff breaks, which also affects end
users. A recent BF failure[2] that looks like one of those jangled my
nerves when I pushed a commit, so I looked into a new theory on how to
fix it. First, let me
> On 5 Jun 2024, at 04:39, Nathan Bossart wrote:
>
> On Wed, May 15, 2024 at 03:21:36PM -0500, Nathan Bossart wrote:
>> Nice! I'll plan on taking a closer look at this one.
>
> LGTM. I've marked the commitfest entry as ready-for-committer.
Thanks for review, committed.
--
Daniel Gustafsson
On Sat, Aug 31, 2024 at 9:30 PM Junwang Zhao wrote:
> @@ -1241,7 +1244,7 @@ GetCachedPlan(CachedPlanSource *plansource,
> ParamListInfo boundParams,
> if (customplan)
> {
> /* Build a custom plan */
> - plan = BuildCachedPlan(plansource, qlist, boundParams, queryEnv);
> + plan = BuildCachedP
On Fri, Aug 30, 2024 at 4:32 PM Amit Langote wrote:
> On Thu, Aug 22, 2024 at 12:44 PM Amit Langote wrote:
> > On Thu, Aug 22, 2024 at 11:02 jian he wrote:
> >> On Tue, Jul 30, 2024 at 12:59 PM Amit Langote
> >> wrote:
> >> > On Fri, Jul 26, 2024 at 11:19 PM jian he
> >> > wrote:
> >> > > {
Hi. Thanks for addressing my previous review comments.
Here are some review comments for v44-0001.
==
Commit message.
1.
Because such synced slots are typically considered not
active (for them to be later considered as inactive) as they don't
perform logical decoding to produce the changes.
> On 23 Aug 2024, at 01:56, Michael Paquier wrote:
>
> On Thu, Aug 22, 2024 at 11:13:15PM +0200, Daniel Gustafsson wrote:
>> On 22 Aug 2024, at 02:31, Michael Paquier wrote:
>>> Just do it :)
>>
>> That's my plan, I wanted to wait a bit to see if anyone else chimed in with
>> concerns.
>
> Coo
On 26.08.24 19:54, Heikki Linnakangas wrote:
On 26/08/2024 20:38, Peter Eisentraut wrote:
On 24.08.24 15:55, Heikki Linnakangas wrote:
Come to think of it, the pg_get_user_name() function is just a thin
wrapper around getpwuid_r(). It doesn't provide a lot of value.
Perhaps we should remove pg
70 matches
Mail list logo
