2024-01 Commitfest.
Hi, this patch was marked in CF as "Needs Review", but there has been
no activity on this thread for 9+ months.
Since there seems not much interest, I have changed the status to
"Returned with Feedback" [1]. Feel free to propose a stronger use case
for the patch and add an ent
Hm, in an optimized build using kernel perf I see this. But I don't
know how to find what the call sites are for LWLockAcquire/Release. If
it's the locks on pgproc that would be kind of bad.
I wonder if I should be gathering horizons once in the
PrecommitActions and then just using those for every
On Wed, 5 Apr 2023 at 13:42, Andres Freund wrote:
>
> Somehow it doesn't feel right to use vac_update_relstats() in
> heapam_handler.c.
>
> I also don't like that your patch references
> heapam_relation_nontransactional_truncate in AddNewRelationTuple() - we
> shouldn't add more comments piercing
On Fri, Apr 14, 2023 at 10:47 AM Andres Freund wrote:
> I don't think it's outright wrong, but it is very confusing what it relates
> to. For some reason I tried to "attach" the parenthetical to the "otherwise",
> which doesn't make a whole lot of sense. How about:
I suppose that it doesn't matte
Hi,
On 2023-04-14 10:05:08 -0400, Greg Stark wrote:
> On Thu, 13 Apr 2023 at 13:01, Peter Geoghegan wrote:
> >
> > On Thu, Apr 13, 2023 at 9:45 AM Robert Haas wrote:
> > >
> > > On Wed, Apr 12, 2023 at 4:23 PM Greg Stark wrote:
> > > > Am I crazy or is the parenthetical comment there exactly ba
On Fri, Apr 14, 2023 at 7:05 AM Greg Stark wrote:
> But I'm saying the parenthetical part is not just confusing, it's
> outright wrong. I guess that just means the first half was so
> confusing it confused not only the reader but the author too.
I knew that that was what you meant. I agree that i
On Thu, 13 Apr 2023 at 13:01, Peter Geoghegan wrote:
>
> On Thu, Apr 13, 2023 at 9:45 AM Robert Haas wrote:
> >
> > On Wed, Apr 12, 2023 at 4:23 PM Greg Stark wrote:
> > > Am I crazy or is the parenthetical comment there exactly backwards? If
> > > the horizon is *more recent* then fewer tuples
On Thu, Apr 13, 2023 at 9:45 AM Robert Haas wrote:
>
> On Wed, Apr 12, 2023 at 4:23 PM Greg Stark wrote:
> > Am I crazy or is the parenthetical comment there exactly backwards? If
> > the horizon is *more recent* then fewer tuples are *non*-removable.
> > I.e. *more* tuples are removable, no?
>
>
On Wed, Apr 12, 2023 at 4:23 PM Greg Stark wrote:
> I'm trying to wrap my head around GetOldestNonRemovableTransactionId()
> and whether it's the right thing here. This comment is not helping me:
>
> /*
> * Return the oldest XID for which deleted tuples must be preserved in the
> * passed table.
On Wed, 5 Apr 2023 at 13:42, Andres Freund wrote:
>
> ISTM that the problem here really is that you're relying on RecentXmin, rather
> than computing something more accurate. Why not use
> GetOldestNonRemovableTransactionId(rel) - It's a bit more expensive, but I
> don't think it'll matter compare
On Wed, 5 Apr 2023 at 13:42, Andres Freund wrote:
>
> Not if you determine a relation specific xmin, and the relation is not a
> shared relation.
>
> ISTM that the problem here really is that you're relying on RecentXmin, rather
> than computing something more accurate. Why not use
> GetOldestNonR
Hi,
On 2023-04-05 13:26:53 -0400, Greg Stark wrote:
> On Wed, 5 Apr 2023 at 11:15, Andres Freund wrote:
> >
> > The freebsd test that failed is running tests in parallel, against an
> > existing
> > cluster. In that case it's expected that there's some concurrency.
> >
> > Why does this cause yo
On Wed, 5 Apr 2023 at 11:15, Andres Freund wrote:
>
> The freebsd test that failed is running tests in parallel, against an existing
> cluster. In that case it's expected that there's some concurrency.
>
> Why does this cause your tests to fail? They're in separate databases, so the
> visibility e
Hi,
On 2023-04-05 10:19:10 -0400, Greg Stark wrote:
> On Wed, 5 Apr 2023 at 01:41, Greg Stark wrote:
> >
> > On Wed, 29 Mar 2023 at 17:48, Justin Pryzby wrote:
> > >
> > > The patch still occasionally fails its tests under freebsd.
> > > https://cirrus-ci.com/github/postgresql-cfbot/postgresql/c
On Wed, 5 Apr 2023 at 01:41, Greg Stark wrote:
>
> On Wed, 29 Mar 2023 at 17:48, Justin Pryzby wrote:
> >
> > The patch still occasionally fails its tests under freebsd.
> > https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/42/3358
>
> I wonder if some other test is behaving dif
On Wed, 29 Mar 2023 at 17:48, Justin Pryzby wrote:
>
> The patch still occasionally fails its tests under freebsd.
> https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/42/3358
So on the one hand, I don't think the plan is to actually commit the
tests. They're very specific to one
On Thu, Feb 2, 2023, 15:47 Alvaro Herrera wrote:
> Are you still at this? CFbot says the meson tests failed last time for
> some reason:
> http://commitfest.cputube.org/greg-stark.html
On Sat, Feb 04, 2023 at 05:12:36PM +0100, Greg Stark wrote:
> I think that was spurious. It looked good when we
Hi,
On 2023-02-05 15:30:57 -0800, Andres Freund wrote:
> Hm, I suspect the problem is that we didn't shut down the server due to
> the error, so the log file was changing while cirrus was trying to
> upload.
Pushed a fix for that.
Greetings,
Andres Freund
Hi,
On 2023-02-04 17:12:36 +0100, Greg Stark wrote:
> I think that was spurious. It looked good when we looked at it yesterday.
> The rest that failed seemed unrelated and was also taking on my SSL patch
> too.
I don't think the SSL failures are related to the failure of this
patch. That was in o
I think that was spurious. It looked good when we looked at it yesterday.
The rest that failed seemed unrelated and was also taking on my SSL patch
too.
I talked to Andres about the possibility of torn reads in the pg_class
stats but those are all 4-byte columns so probably safe. And in any case
t
On 2022-Dec-13, Greg Stark wrote:
> So here I've done it that way. It is a bit of an unfortunate layering
> since it means the heapam_handler is doing the catalog change but it
> does seem inconvenient to pass relfrozenxid etc back up and have the
> caller make the changes when there are no other
On Wed, Dec 14, 2022 at 4:44 PM Greg Stark wrote:
> > You do have to lock a table in order to update its pg_class row,
> > though, whether the table is temporary or not. Otherwise, another
> > session could drop it while you're doing something with it, after
> > which bad things would happen.
>
>
> You do have to lock a table in order to update its pg_class row,
> though, whether the table is temporary or not. Otherwise, another
> session could drop it while you're doing something with it, after
> which bad things would happen.
I was responding to this from Andres:
> Is that actually true
On Wed, Dec 14, 2022 at 1:18 PM Greg Stark wrote:
> So I don't see any evidence we skip any locking on pg_class when doing
> updates on rows for temporary tables.
I don't know what this means. You don't have to lock pg_class to
update rows in any table, whether temporary or otherwise.
You do hav
On Sat, 5 Nov 2022 at 15:34, Tom Lane wrote:
>
> Greg Stark writes:
> > Simple Rebase
>
> I took a little bit of a look through these.
>
> * I find 0001 a bit scary, specifically that it's decided it's
> okay to apply extract_autovac_opts, pgstat_fetch_stat_tabentry_ext,
> and especially relation
On Wed, 7 Dec 2022 at 22:02, Greg Stark wrote:
> > Seems like this should just be in
> > heapam_relation_nontransactional_truncate()?
So here I've done it that way. It is a bit of an unfortunate layering
since it means the heapam_handler is doing the catalog change but it
does seem inconvenient t
On Thu, 1 Dec 2022 at 14:18, Andres Freund wrote:
>
> I find it problematic that ResetVacStats() bypasses tableam. Normal vacuums
> etc go through tableam but you put a ResetVacStats() besides each call to
> table_relation_nontransactional_truncate(). Seems like this should just be in
> heapam_re
Hi,
On 2022-12-06 14:50:34 -0500, Greg Stark wrote:
> On Tue, 6 Dec 2022 at 13:59, Andres Freund wrote:
> > On 2022-12-06 13:47:39 -0500, Greg Stark wrote:
> > > So I talked about this patch with Ronan Dunklau and he had a good
> > > question Why are we maintaining relfrozenxid and relmin
On Tue, 6 Dec 2022 at 13:59, Andres Freund wrote:
>
> Hi,
>
> On 2022-12-06 13:47:39 -0500, Greg Stark wrote:
> > So I talked about this patch with Ronan Dunklau and he had a good
> > question Why are we maintaining relfrozenxid and relminmxid in
> > pg_class for temporary tables at all? A
Hi,
On 2022-12-06 13:47:39 -0500, Greg Stark wrote:
> So I talked about this patch with Ronan Dunklau and he had a good
> question Why are we maintaining relfrozenxid and relminmxid in
> pg_class for temporary tables at all? Autovacuum can't use them and
> other sessions won't care about t
So I talked about this patch with Ronan Dunklau and he had a good
question Why are we maintaining relfrozenxid and relminmxid in
pg_class for temporary tables at all? Autovacuum can't use them and
other sessions won't care about them. The only session that might care
about them is the one a
On Thu, 1 Dec 2022 at 14:18, Andres Freund wrote:
>
> Hi,
>
> On 2022-12-01 11:13:01 -0500, Greg Stark wrote:
> > On Sat, 5 Nov 2022 at 11:34, Tom Lane wrote:
> >
> > > * I find 0001 a bit scary, specifically that it's decided it's
> > > okay to apply extract_autovac_opts, pgstat_fetch_stat_taben
Hi,
On 2022-12-01 11:13:01 -0500, Greg Stark wrote:
> On Sat, 5 Nov 2022 at 11:34, Tom Lane wrote:
> >
> > Greg Stark writes:
> > > Simple Rebase
> >
> > I took a little bit of a look through these.
> >
> > * I find 0001 a bit scary, specifically that it's decided it's
> > okay to apply extract_
On Sat, 5 Nov 2022 at 11:34, Tom Lane wrote:
>
> Greg Stark writes:
> > Simple Rebase
>
> I took a little bit of a look through these.
>
> * I find 0001 a bit scary, specifically that it's decided it's
> okay to apply extract_autovac_opts, pgstat_fetch_stat_tabentry_ext,
> and especially relation
Greg Stark writes:
> Simple Rebase
I took a little bit of a look through these.
* I find 0001 a bit scary, specifically that it's decided it's
okay to apply extract_autovac_opts, pgstat_fetch_stat_tabentry_ext,
and especially relation_needs_vacanalyze to another session's
temp table. How safe i
On Sun, 8 Nov 2020 at 18:19, Greg Stark wrote:
>
> We had an outage caused by transaction wraparound. And yes, one of the
> first things I did on this site was check that we didn't have any
> databases that were in danger of wraparound.
Fwiw this patch has been in "Ready for Committer" state sinc
Simple Rebase
From 8dfed1a64308a84cc15679e09af69ca6989b608b Mon Sep 17 00:00:00 2001
From: Greg Stark
Date: Thu, 31 Mar 2022 15:50:02 -0400
Subject: [PATCH v7 3/3] Add test for truncating temp tables advancing
relfrozenxid
This test depends on other transactions not running at the same time
so t
So here's an updated patch.
I had to add a public method to multixact.c to expose the locally
calculated OldestMultiXactId. It's possible we could use something
even tighter (like the current next mxid since we're about to commit)
but I didn't see a point in going further and it would have become
On Thu, 31 Mar 2022 at 16:05, Greg Stark wrote:
>
> I haven't wrapped my head around multixacts yet. It's complicated by
> this same codepath being used for truncates of regular tables that
> were created in the same transaction.
So my best idea so far is to actually special-case the temp table c
I've updated the patches.
Adding the assertion actually turned up a corner case where RecentXmin
was *not* set. If you lock a temporary table and that's the only thing
you do in a transaction then the flag is set indicating you've used
the temp schema but you never take a snapshot :(
I also split
Hi,
On 2022-03-29 19:51:26 -0400, Greg Stark wrote:
> On Mon, 28 Mar 2022 at 16:30, Andres Freund wrote:
> >
> > > Make ON COMMIT DELETE ROWS reset relfrozenxmin and other table stats
> > > like normal truncate. Otherwise even typical short-lived transactions
> > > using temporary tab
On Tue, Mar 29, 2022 at 4:52 PM Greg Stark wrote:
> On Mon, 28 Mar 2022 at 16:30, Andres Freund wrote:
> >
> > > Make ON COMMIT DELETE ROWS reset relfrozenxmin and other table
> stats
> > > like normal truncate. Otherwise even typical short-lived
> transactions
> > > using temporary
On Mon, 28 Mar 2022 at 16:30, Andres Freund wrote:
>
> > Make ON COMMIT DELETE ROWS reset relfrozenxmin and other table stats
> > like normal truncate. Otherwise even typical short-lived transactions
> > using temporary tables can easily cause them to reach relfrozenxid.
>
> Might be w
Hi,
On 2022-03-28 16:11:55 -0400, Greg Stark wrote:
> From 4515075b644d1e38920eb5bdaaa898e1698510a8 Mon Sep 17 00:00:00 2001
> From: Greg Stark
> Date: Tue, 22 Mar 2022 15:51:32 -0400
> Subject: [PATCH v4 1/2] Update relfrozenxmin when truncating temp tables
>
> Make ON COMMIT DELETE ROW
I had to rebase this again after Tom's cleanup of heap.c removing some includes.
I had to re-add snapmgr to access RecentXmin. I occurs to me to ask
whether RecentXmin is actually guaranteed to be set. I haven't
checked. I thought it was set when the first snapshot was taken and
presumably even if
Here's a rebased patch. I split the test into a separate patch that I
would lean to dropping. But at least it applies now.
I did look into pg_stat_get_backend_pid() and I guess it would work
but going through the stats mechanism does seem like going the long
way around since we're already looking
No problem, I can update the patch and check on the fuzz.
But the actual conflict is just in the test and I'm not sure it's
really worth having a test at all. It's testing a pretty low level
detail. So I'm leaning toward fixing the conflict by just ripping the
test out.
Nathan also pointed out th
Hi,
On 2021-10-12 18:04:35 -0400, Greg Stark wrote:
> Here's an updated patch.
Unfortunately it doesn't apply anymore these days:
http://cfbot.cputube.org/patch_37_3358.log
Marked as waiting-on-author.
Greetings,
Andres Freund
On 10/12/21, 3:07 PM, "Greg Stark" wrote:
> Here's an updated patch. I added some warning messages to autovacuum.
I think this is useful optimization, and I intend to review the patch
more closely soon. It looks reasonable to me after a quick glance.
> One thing I learned trying to debug this s
Here's an updated patch. I added some warning messages to autovacuum.
One thing I learned trying to debug this situation in production is
that it's nigh impossible to find the pid of the session using a
temporary schema. The number in the schema refers to the backendId in
the sinval stuff for whic
On Tue, Nov 10, 2020 at 04:10:57PM +0900, Masahiko Sawada wrote:
> On Mon, Nov 9, 2020 at 3:23 PM Greg Stark wrote:
> > On Mon, 9 Nov 2020 at 00:17, Noah Misch wrote:
> > > > @@ -3340,6 +3383,7 @@ heap_truncate_one_rel(Relation rel)
> > > >
> > > > /* Truncate the underlying relation */
>
On Mon, Nov 9, 2020 at 3:23 PM Greg Stark wrote:
>
> On Mon, 9 Nov 2020 at 00:17, Noah Misch wrote:
> >
> > > 2) adding the dependency on heapam.h to heap.c makes sense because of
> > > heap_inplace_update bt it may be a bit annoying because I suspect
> > > that's a useful sanity check that the t
On Mon, 9 Nov 2020 at 00:17, Noah Misch wrote:
>
> > 2) adding the dependency on heapam.h to heap.c makes sense because of
> > heap_inplace_update bt it may be a bit annoying because I suspect
> > that's a useful sanity check that the tableam stuff hasn't been
> > bypassed
>
> That is not terrible
On Sun, Nov 08, 2020 at 06:19:57PM -0500, Greg Stark wrote:
> However in the case of ON COMMIT DELETE ROWS we can do better. Why not
> just reset the relfrozenxid and other stats as if the table was
> freshly created when it's truncated?
That concept is sound.
> 1) Should we update relpages and r
54 matches
Mail list logo
