> I can see it being confusing. What about this wording to make it more
> clear when the field is
> updated?
here are both of the changes in v4.
--
Sami Imseih
Amazon Web Services (AWS)
v4-0001-Clarify-when-aborted-rows-are-tracked-for-tuple-r.patch
Description: Binary data
On Thu, Apr 17, 2025 at 11:13 PM David Rowley wrote:
>
> On Sat, 12 Apr 2025 at 07:33, Sami Imseih wrote:
> > What do you think of the attached?
>
> I looked at the v3 patch and I'm having trouble getting excited about it.
>
> I'd say this part is misleading:
>
> @@ -3956,7 +3961,8 @@ description
On Sat, 12 Apr 2025 at 07:33, Sami Imseih wrote:
> What do you think of the attached?
I looked at the v3 patch and I'm having trouble getting excited about it.
I'd say this part is misleading:
@@ -3956,7 +3961,8 @@ description | Waiting for a newly initialized
WAL file to reach durable storage
On Fri, Apr 11, 2025 at 5:19 PM Sami Imseih wrote:
> > WFM. Though is there a reason to avoid adding the "why" of the
> exception for n_mod_since_analyze?
>
> I did think about that. I thought it will be understood that since
> this is a field that deals with ANALYZE,
> it will be understood tha
> WFM. Though is there a reason to avoid adding the "why" of the exception for
> n_mod_since_analyze?
I did think about that. I thought it will be understood that since
this is a field that deals with ANALYZE,
it will be understood that only committed changes matter here, and not
worth adding mo
On Fri, Apr 11, 2025 at 12:33 PM Sami Imseih wrote:
>
> > I'm also thinking to reword n_tup_upd, something like:
> >
> > Total number of rows updated. Subsets of these updates are also tracked
> in n_tup_hot_upd and n_tup_newpage_upd to facilitate performance monitoring.
>
> I think the current
I spent some time thinking about this today.
> "The tuple counters below, except where noted, are incremented even if the
> transaction aborts."
I like this idea, and I think it fits good as a blurb under "27.2.2.
Viewing Statistics"
I suggest a slight re-write however.
+
+ An aborted tran
On Thu, Apr 10, 2025 at 5:38 PM Sami Imseih wrote:
> > On Fri, 11 Apr 2025 at 02:01, Sami Imseih wrote:
> > > I created an entry for the July CF
> > > https://commitfest.postgresql.org/patch/5691/
> > >
> > > ... and I realized I forgot to include David's code comment patch
> yesterday,
> > > Re
> On Fri, 11 Apr 2025 at 02:01, Sami Imseih wrote:
> > I created an entry for the July CF
> > https://commitfest.postgresql.org/patch/5691/
> >
> > ... and I realized I forgot to include David's code comment patch yesterday,
> > Reattaching both patches.
>
> I've pushed the code comment patch.
>
>
On Fri, 11 Apr 2025 at 02:01, Sami Imseih wrote:
> I created an entry for the July CF
> https://commitfest.postgresql.org/patch/5691/
>
> ... and I realized I forgot to include David's code comment patch yesterday,
> Reattaching both patches.
I've pushed the code comment patch.
For the docs patc
On Wed, Apr 9, 2025, at 1:57 PM, Sami Imseih wrote:
> I came across what appears to be incorrect behavior in the
> pg_stat_all_tables.n_ins_since_vacuum
> counter after a rollback.
This is *not* an oversight. It is by design. See commit b07642dbcd8d. The
documentation says
Estimated number of r
On Wednesday, April 9, 2025, Sami Imseih wrote:
>
> In other words, the reason n_ins_since_vacuum was introduced is to freeze
> (committed) rows, so it should not need to track dead rows to do what it
> intends
> to do.
>
n_ins_since_vacuum was introduced to indicate how many tuples a vacuum
wou
I created an entry for the July CF
https://commitfest.postgresql.org/patch/5691/
... and I realized I forgot to include David's code comment patch yesterday,
Reattaching both patches.
--
Sami Imseih
Amazon Web Services (AWS)
v1-0001-Add-code-comment-for-ins_since_vacuum.patch
Description: Bina
>> >> What I am saying is n_ins_since_vacuum should not account for aborted
>> >> inserts.
>> >
>> > It does and from what I can see it should. You need to explain why it
>> > should not. More importantly, convincingly enough to change five year old
>> > behavior.
>>
>> n_ins_since_vacuum was
> One possible bad behaviour that this could cause is if
> autovacuum_vacuum_insert_scale_factor was set lower than
> autovacuum_vacuum_scale_factor is that we could end up performing a
> vacuum for inserts when all we've got is dead tuples from aborted
> inserts.
Thanks for pointing this out!
>
On Thu, 10 Apr 2025 at 10:54, Sami Imseih wrote:
> Fair enough, and I think I got my answers from this thread. This
> design decision was not
> discussed in the thread for b07642dbcd8.
This discussion is mostly settled down now, but...
I also went through that thread to see if it was mentioned a
> Except there isn’t some singular provably correct value here. Today’s
> behavior (considering dead tuples)
> is not intrinsically wrong nor correct, and neither is what you propose
> (ignoring the dead tuples).
> The fact that those dead tuples get removed regardless is a point in favor of
>
On Wed, Apr 9, 2025, 12:56 Sami Imseih wrote:
>
> What I am saying is n_ins_since_vacuum should not account for aborted
> inserts.
>
It does and from what I can see it should. You need to explain why it
should not. More importantly, convincingly enough to change five year old
behavior.
You sh
On Wednesday, April 9, 2025, Sami Imseih wrote:
>
> > What is the use case for that behavior? Perhaps you have one, but until
> you make it explicit, it is hard for others to get behind your proposal.
>
> The point is to ensure that the pg_stats fields that autovacuum uses
> are supplied the cor
Hi,
On 2025-04-09 17:05:39 -0500, Sami Imseih wrote:
> On Wed, Apr 9, 2025 at 4:23 PM Mark Dilger
> wrote:
> >>
> >> In other words, the reason n_ins_since_vacuum was introduced is to freeze
> >> (committed) rows, so it should not need to track dead rows to do what it
> >> intends
> >> to do.
>
On Wed, Apr 9, 2025 at 4:23 PM Mark Dilger wrote:
>>
>> In other words, the reason n_ins_since_vacuum was introduced is to freeze
>> (committed) rows, so it should not need to track dead rows to do what it
>> intends
>> to do.
>
>
> Wouldn't that result in the rather strange behavior that 1 milli
>
> In other words, the reason n_ins_since_vacuum was introduced is to freeze
> (committed) rows, so it should not need to track dead rows to do what it
> intends
> to do.
>
Wouldn't that result in the rather strange behavior that 1 million dead
rows might trigger a vacuum due to one threshold, 1
On Wed, Apr 9, 2025 at 1:30 PM Sami Imseih wrote:
> >> What I am saying is n_ins_since_vacuum should not account for aborted
> inserts.
> >
> > It does and from what I can see it should. You need to explain why it
> should not. More importantly, convincingly enough to change five year old
> beh
>> What I am saying is n_ins_since_vacuum should not account for aborted
>> inserts.
>
> It does and from what I can see it should. You need to explain why it should
> not. More importantly, convincingly enough to change five year old behavior.
n_ins_since_vacuum was introduced to trigger auto
On Wed, Apr 9, 2025, 12:39 Sami Imseih wrote:
>
> They will differ because n_tup_ins keeps increasing, while
> n_ins_since_vacuum is
> reset after a vacuum. The issue I see is that n_ins_since_vacuum should
> only
> reflect the number of newly inserted rows that are eligible for
> freezing, as de
> So why is it important we not account for the aborted insert in both
> n_ins_since_vacuum and n_dead_tup?
I am saying n_dead_tup should continue to account for n_dead_tup. I am
not saying it should not.
What I am saying is n_ins_since_vacuum should not account for aborted inserts.
> When would
On Wed, Apr 9, 2025, 11:57 Sami Imseih wrote:
> > Forget original purpose, is there presently a bug or not?
>
> Yes, there is a bug. Accounting rows inserted as part of an aborted
> transaction in
> n_ins_since_vacuum is not correct, since the same rows are being
> accounted for with n_dead_tup.
> The issue I see is that n_ins_since_vacuum should only
> reflect the number of newly inserted rows that are eligible for
> freezing, as described
> in pgstat_report_vacuum [0]
>
> [0]
> https://github.com/postgres/postgres/blob/master/src/backend/utils/activity/pgstat_relation.c#L238-L247
For t
> If we went with your suggestion, I think the final n_ins_since_vacuum column
> would be 2. Do you think the n_tup_ins should also be 2?
n_ins_since_vacuum should be 2 and n_tup_ins should be 10.
A user tracks how many inserts they performed with n_tup_ins
to measure load/activity on the d
>
> Yes, there is a bug. Accounting rows inserted as part of an aborted
> transaction in
> n_ins_since_vacuum is not correct, since the same rows are being
> accounted for with n_dead_tup.
>
If I create a table with autovacuum_enabled=false, insert rows (some of
which abort), and check the stats,
On Wed, Apr 9, 2025 at 11:32 AM Sami Imseih wrote:
> The purpose of b07642dbcd8d is to trigger autovacuum for append only/mainly
> workloads that don't generate dead tuples.
>
>
Why does counting or not counting the dead tuples matter? Forget original
purpose, is there presently a bug or not? B
> Forget original purpose, is there presently a bug or not?
Yes, there is a bug. Accounting rows inserted as part of an aborted
transaction in
n_ins_since_vacuum is not correct, since the same rows are being
accounted for with n_dead_tup.
> Between the two options the one where we count dead tupl
> This is *not* an oversight. It is by design. See commit b07642dbcd8d. The
> documentation says
I don't see in the commit message why inserts in an aborted transaction
must count towards n_ins_since_vacuum.
> Estimated number of rows inserted since this table was last vacuumed
>
> Those rows w
Hi,
I came across what appears to be incorrect behavior in the
pg_stat_all_tables.n_ins_since_vacuum
counter after a rollback.
As shown below, the first two inserts of 1,000 rows were rolled back,
yet they are still counted toward
n_ins_since_vacuum.Consequently, they influence the vacuum insert
34 matches
Mail list logo
