pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedPROC_IN_ANALYZE stillborn 13 years ago
18+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2020-08-07 cea3d55898 Remove PROC_IN_ANALYZE and derived flags
* PROC_IN_ANALYZE stillborn 13 years ago
@ 2020-08-05 23:55 Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 18+ messages in thread
From: Alvaro Herrera @ 2020-08-05 23:55 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; simon@2ndquadrant.com
Back in the 8.3 cycle (2007) when the autovacuum launcher/worker split
was done, we annoyed people because it blocked DDL. That led to an
effort to cancel autovac automatically when that was detected, by Simon
Riggs.
https://postgr.es/m/1191526327.4223.204.camel@ebony.site
https://postgr.es/m/1192129897.4233.433.camel@ebony.site
I was fixated on only cancelling when it was ANALYZE, to avoid losing
any VACUUM work.
https://postgr.es/m/20071025164150.GF23566@alvh.no-ip.org
That turned into some flags for PGPROC to detect whether a process was
ANALYZE, and cancel only those.
https://postgr.es/m/20071024151328.GG6559@alvh.no-ip.org
Commit:
https://postgr.es/m/20071024205536.CB425754229@cvs.postgresql.org
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=745c1b2c2ab
However, I was outvoted, so we do not limit cancellation to analyze.
Patch and discussion: https://postgr.es/m/20071025164150.GF23566@alvh.no-ip.org
Commit:
https://postgr.es/m/20071026204510.AA02E754229@cvs.postgresql.org
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=acac68b2bca
... which means the flag I had added two days earlier has never been
used for anything. We've carried the flag forward to this day for
almost 13 years, dutifully turning it on and off ... but never checking
it anywhere.
I propose to remove it, as in the attached patch.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Remove-PROC_IN_ANALYZE.patch (text/x-diff, 3.6K) download
From f96b28defe856e284a44b207c2016c72a48a2ff2 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 5 Aug 2020 18:57:50 -0400
Subject: [PATCH] Remove PROC_IN_ANALYZE
---
src/backend/commands/analyze.c | 13 +------------
src/include/storage/proc.h | 3 +--
src/include/storage/procarray.h | 7 -------
3 files changed, 2 insertions(+), 21 deletions(-)
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 924ef37c81..e0fa73ba79 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -247,11 +247,8 @@ analyze_rel(Oid relid, RangeVar *relation,
}
/*
- * OK, let's do it. First let other backends know I'm in ANALYZE.
+ * OK, let's do it. First, initialize progress reporting.
*/
- LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
- MyPgXact->vacuumFlags |= PROC_IN_ANALYZE;
- LWLockRelease(ProcArrayLock);
pgstat_progress_start_command(PROGRESS_COMMAND_ANALYZE,
RelationGetRelid(onerel));
@@ -279,14 +276,6 @@ analyze_rel(Oid relid, RangeVar *relation,
relation_close(onerel, NoLock);
pgstat_progress_end_command();
-
- /*
- * Reset my PGXACT flag. Note: we need this here, and not in vacuum_rel,
- * because the vacuum flag is cleared by the end-of-xact code.
- */
- LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
- MyPgXact->vacuumFlags &= ~PROC_IN_ANALYZE;
- LWLockRelease(ProcArrayLock);
}
/*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index b20e2ad4f6..5ceb2494ba 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -52,7 +52,6 @@ struct XidCache
*/
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
-#define PROC_IN_ANALYZE 0x04 /* currently running analyze */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
#define PROC_IN_LOGICAL_DECODING 0x10 /* currently doing logical
* decoding outside xact */
@@ -60,7 +59,7 @@ struct XidCache
/* flags reset at EOXact */
#define PROC_VACUUM_STATE_MASK \
- (PROC_IN_VACUUM | PROC_IN_ANALYZE | PROC_VACUUM_FOR_WRAPAROUND)
+ (PROC_IN_VACUUM | PROC_VACUUM_FOR_WRAPAROUND)
/*
* We allow a small number of "weak" relation locks (AccessShareLock,
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index a5c7d0c064..01040d76e1 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -29,8 +29,6 @@
*/
#define PROCARRAY_VACUUM_FLAG 0x02 /* currently running lazy
* vacuum */
-#define PROCARRAY_ANALYZE_FLAG 0x04 /* currently running
- * analyze */
#define PROCARRAY_LOGICAL_DECODING_FLAG 0x10 /* currently doing logical
* decoding outside xact */
@@ -42,7 +40,6 @@
* have no corresponding PROC flag equivalent.
*/
#define PROCARRAY_PROC_FLAGS_MASK (PROCARRAY_VACUUM_FLAG | \
- PROCARRAY_ANALYZE_FLAG | \
PROCARRAY_LOGICAL_DECODING_FLAG)
/* Use the following flags as an input "flags" to GetOldestXmin function */
@@ -50,10 +47,6 @@
#define PROCARRAY_FLAGS_DEFAULT PROCARRAY_LOGICAL_DECODING_FLAG
/* Ignore vacuum backends */
#define PROCARRAY_FLAGS_VACUUM PROCARRAY_FLAGS_DEFAULT | PROCARRAY_VACUUM_FLAG
-/* Ignore analyze backends */
-#define PROCARRAY_FLAGS_ANALYZE PROCARRAY_FLAGS_DEFAULT | PROCARRAY_ANALYZE_FLAG
-/* Ignore both vacuum and analyze backends */
-#define PROCARRAY_FLAGS_VACUUM_ANALYZE PROCARRAY_FLAGS_DEFAULT | PROCARRAY_VACUUM_FLAG | PROCARRAY_ANALYZE_FLAG
extern Size ProcArrayShmemSize(void);
extern void CreateSharedProcArray(void);
--
2.20.1
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2020-08-06 01:07 ` Andres Freund <andres@anarazel.de>
2020-08-06 08:17 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Simon Riggs <simon@2ndquadrant.com>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-07 21:35 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 3 replies; 18+ messages in thread
From: Andres Freund @ 2020-08-06 01:07 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; simon@2ndquadrant.com
Hi,
On 2020-08-05 19:55:49 -0400, Alvaro Herrera wrote:
> Back in the 8.3 cycle (2007) when the autovacuum launcher/worker split
> was done, we annoyed people because it blocked DDL. That led to an
> effort to cancel autovac automatically when that was detected, by Simon
> Riggs.
> https://postgr.es/m/1191526327.4223.204.camel@ebony.site
> https://postgr.es/m/1192129897.4233.433.camel@ebony.site
>
> I was fixated on only cancelling when it was ANALYZE, to avoid losing
> any VACUUM work.
> https://postgr.es/m/20071025164150.GF23566@alvh.no-ip.org
> That turned into some flags for PGPROC to detect whether a process was
> ANALYZE, and cancel only those.
> https://postgr.es/m/20071024151328.GG6559@alvh.no-ip.org
> Commit:
> https://postgr.es/m/20071024205536.CB425754229@cvs.postgresql.org
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=745c1b2c2ab
>
> However, I was outvoted, so we do not limit cancellation to analyze.
> Patch and discussion: https://postgr.es/m/20071025164150.GF23566@alvh.no-ip.org
> Commit:
> https://postgr.es/m/20071026204510.AA02E754229@cvs.postgresql.org
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=acac68b2bca
>
> ... which means the flag I had added two days earlier has never been
> used for anything. We've carried the flag forward to this day for
> almost 13 years, dutifully turning it on and off ... but never checking
> it anywhere.
>
> I propose to remove it, as in the attached patch.
I'm mildly against that, because I'd really like to start making use of
the flag. Not so much for cancellations, but to avoid the drastic impact
analyze has on bloat. In OLTP workloads with big tables, and without
disabled cost limiting for analyze (or slow IO), the snapshot that
analyze holds is often by far the transaction with the oldest xmin.
It's not entirely trivial to fix (just ignoring it could lead to
detoasting issues), but also not that.
Only mildly against because it'd not be hard to reintroduce once we need
it.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
@ 2020-08-06 08:17 ` Simon Riggs <simon@2ndquadrant.com>
2 siblings, 0 replies; 18+ messages in thread
From: Simon Riggs @ 2020-08-06 08:17 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
On Thu, 6 Aug 2020 at 02:07, Andres Freund <andres@anarazel.de> wrote:
>
> On 2020-08-05 19:55:49 -0400, Alvaro Herrera wrote:
> > ... which means the flag I had added two days earlier has never been
> > used for anything. We've carried the flag forward to this day for
> > almost 13 years, dutifully turning it on and off ... but never checking
> > it anywhere.
> >
> > I propose to remove it, as in the attached patch.
>
> I'm mildly against that, because I'd really like to start making use of
> the flag. Not so much for cancellations, but to avoid the drastic impact
> analyze has on bloat. In OLTP workloads with big tables, and without
> disabled cost limiting for analyze (or slow IO), the snapshot that
> analyze holds is often by far the transaction with the oldest xmin.
>
> It's not entirely trivial to fix (just ignoring it could lead to
> detoasting issues), but also not that.
>
> Only mildly against because it'd not be hard to reintroduce once we need
> it.
>
Good points, both.
The most obvious way to avoid long analyze snapshots is to make the
analysis take multiple snapshots as it runs, rather than try to invent some
clever way of ignoring the analyze snapshots (which as Alvaro points out,
we never did). All we need to do is to have an analyze snapshot last for at
most N rows, but keep scanning until we have the desired sample size. Doing
that would mean the analyze sample wouldn't come from a single snapshot,
but then who cares? There is no requirement for consistency - the sample
would be arguably *more* stable because it comes from multiple points in
time, not just one.
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/;
Mission Critical Databases
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
@ 2020-08-06 18:25 ` Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2 siblings, 1 reply; 18+ messages in thread
From: Robert Haas @ 2020-08-06 18:25 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
On Wed, Aug 5, 2020 at 9:07 PM Andres Freund <andres@anarazel.de> wrote:
> I'm mildly against that, because I'd really like to start making use of
> the flag. Not so much for cancellations, but to avoid the drastic impact
> analyze has on bloat. In OLTP workloads with big tables, and without
> disabled cost limiting for analyze (or slow IO), the snapshot that
> analyze holds is often by far the transaction with the oldest xmin.
>
> It's not entirely trivial to fix (just ignoring it could lead to
> detoasting issues), but also not that.
>
> Only mildly against because it'd not be hard to reintroduce once we need
> it.
I think we should nuke it. It's trivial to reintroduce the flag if we
need it later, if and when somebody's willing to do the associated
work. In the meantime, it adds confusion.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
@ 2020-08-06 18:37 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 18+ messages in thread
From: Tom Lane @ 2020-08-06 18:37 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Aug 5, 2020 at 9:07 PM Andres Freund <andres@anarazel.de> wrote:
>> Only mildly against because it'd not be hard to reintroduce once we need
>> it.
> I think we should nuke it. It's trivial to reintroduce the flag if we
> need it later, if and when somebody's willing to do the associated
> work. In the meantime, it adds confusion.
+1 for removal. It's not clear to me that we'd ever put it back.
Long-running ANALYZE snapshots are indeed a problem, but Simon's proposal
upthread to just take a new one every so often seems like a much cleaner
and simpler answer than having onlookers assume that it's safe to ignore
ANALYZE processes. (Given that ANALYZE can invoke user-defined functions,
and can be invoked from inside user transactions, any such assumption
seems horribly dangerous.)
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-08-06 18:48 ` Robert Haas <robertmhaas@gmail.com>
2020-08-06 19:11 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 21:26 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
0 siblings, 2 replies; 18+ messages in thread
From: Robert Haas @ 2020-08-06 18:48 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
On Thu, Aug 6, 2020 at 2:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> +1 for removal. It's not clear to me that we'd ever put it back.
> Long-running ANALYZE snapshots are indeed a problem, but Simon's proposal
> upthread to just take a new one every so often seems like a much cleaner
> and simpler answer than having onlookers assume that it's safe to ignore
> ANALYZE processes. (Given that ANALYZE can invoke user-defined functions,
> and can be invoked from inside user transactions, any such assumption
> seems horribly dangerous.
Not to get too far from the proposal on the table of just removing
something that's been unused for a really long time, which stands on
its own merits, but if a particular ANALYZE doesn't invoke any
user-defined functions and isn't run inside a transaction, could we
skip acquiring a snapshot altogether? That's an extremely common case,
though by no means universal.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
@ 2020-08-06 19:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 20:22 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
1 sibling, 1 reply; 18+ messages in thread
From: Tom Lane @ 2020-08-06 19:11 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
Robert Haas <robertmhaas@gmail.com> writes:
> Not to get too far from the proposal on the table of just removing
> something that's been unused for a really long time, which stands on
> its own merits, but if a particular ANALYZE doesn't invoke any
> user-defined functions and isn't run inside a transaction, could we
> skip acquiring a snapshot altogether? That's an extremely common case,
> though by no means universal.
I'm inclined to think not.
(1) Without a snapshot it's hard to make any non-bogus decisions about
which tuples are live and which are dead. Admittedly, with Simon's
proposal the final totals would be spongy anyhow, but at least the
individual decisions produce meaningful answers.
(2) I'm pretty sure there are places in the system that assume that any
reader of a table is using an MVCC snapshot. For instance, didn't you
introduce some such assumptions along with or just after getting rid of
SnapshotNow for catalog scans?
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 19:11 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-08-06 20:22 ` Robert Haas <robertmhaas@gmail.com>
2020-08-06 21:35 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 21:45 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
0 siblings, 2 replies; 18+ messages in thread
From: Robert Haas @ 2020-08-06 20:22 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
On Thu, Aug 6, 2020 at 3:11 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> (1) Without a snapshot it's hard to make any non-bogus decisions about
> which tuples are live and which are dead. Admittedly, with Simon's
> proposal the final totals would be spongy anyhow, but at least the
> individual decisions produce meaningful answers.
I don't think I believe this. It's impossible to make *consistent*
decisions, but it's not difficult to make *non-bogus* decisions.
HeapTupleSatisfiesVacuum() and HeapTupleSatifiesUpdate() both make
such decisions, and neither takes a snapshot argument.
> (2) I'm pretty sure there are places in the system that assume that any
> reader of a table is using an MVCC snapshot. For instance, didn't you
> introduce some such assumptions along with or just after getting rid of
> SnapshotNow for catalog scans?
SnapshotSelf still exists and is still used, and IIRC, it has very
similar semantics to the old SnapshotNow, so I don't think that we
introduced any really general assumptions of this sort. I think the
important part of those changes was that all the code that had
previously used SnapshotNow to examine system catalog tuples for DDL
purposes and catcache lookups and so forth started using an MVCC scan,
which removed one (of many) impediments to concurrent DDL. I think the
fact that we removed SnapshotNow outright rather than just ceasing to
use it for that purpose was mostly so that nobody would accidentally
reintroduce code that used it for the sorts of purposes for which it
had been used previously, and secondarily for code cleanliness.
There's nothing wrong with it fundamentally AFAIK.
It's worth mentioning, I think, that the main problem with SnapshotNow
was that it provided no particular stability. If you did an index scan
under SnapshotNow you might find two copies or no copies of a row
being concurrently updated, rather than exactly one. And that in turn
could cause problems like failure to build a relcache entry. Now, how
important is stability to ANALYZE? If you *either* retake your MVCC
snapshots periodically as you re-scan the table *or* use a non-MVCC
snapshot for the scan, you can get those same kinds of artifacts: you
might see two copies of a just-updated row, or none. Maybe this would
actually *break* something - e.g. could there be code that would get
confused if we sample multiple rows for the same value in a column
that has a UNIQUE index? But I think mostly the consequences would be
that you might get somewhat different results from the statistics.
It's not clear to me that it would even be correct to categorize those
somewhat-different results as "less accurate." Tuples that are
invisible to a query often have performance consequences very similar
to visible tuples, in terms of the query run time.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 19:11 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 20:22 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
@ 2020-08-06 21:35 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-08-07 04:54 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Simon Riggs <simon@2ndquadrant.com>
2020-08-07 18:03 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
1 sibling, 2 replies; 18+ messages in thread
From: Tom Lane @ 2020-08-06 21:35 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
Robert Haas <robertmhaas@gmail.com> writes:
> ... how
> important is stability to ANALYZE? If you *either* retake your MVCC
> snapshots periodically as you re-scan the table *or* use a non-MVCC
> snapshot for the scan, you can get those same kinds of artifacts: you
> might see two copies of a just-updated row, or none. Maybe this would
> actually *break* something - e.g. could there be code that would get
> confused if we sample multiple rows for the same value in a column
> that has a UNIQUE index? But I think mostly the consequences would be
> that you might get somewhat different results from the statistics.
Yeah, that's an excellent point. I can imagine somebody complaining
"this query clearly matches a unique index, why is the planner estimating
multiple rows out?". But most of the time it wouldn't matter much.
(And I think you can get cases like that anyway today.)
> It's not clear to me that it would even be correct to categorize those
> somewhat-different results as "less accurate."
Estimating two rows where the correct answer is one row is clearly
"less accurate". But I suspect you'd have to be quite unlucky to
get such a result in practice from Simon's proposal, as long as we
weren't super-aggressive about changing ANALYZE's snapshot a lot.
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 19:11 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 20:22 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 21:35 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-08-07 04:54 ` Simon Riggs <simon@2ndquadrant.com>
1 sibling, 0 replies; 18+ messages in thread
From: Simon Riggs @ 2020-08-07 04:54 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
On Thu, 6 Aug 2020 at 22:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > ... how
> > important is stability to ANALYZE? If you *either* retake your MVCC
> > snapshots periodically as you re-scan the table *or* use a non-MVCC
> > snapshot for the scan, you can get those same kinds of artifacts: you
> > might see two copies of a just-updated row, or none. Maybe this would
> > actually *break* something - e.g. could there be code that would get
> > confused if we sample multiple rows for the same value in a column
> > that has a UNIQUE index? But I think mostly the consequences would be
> > that you might get somewhat different results from the statistics.
>
> Yeah, that's an excellent point. I can imagine somebody complaining
> "this query clearly matches a unique index, why is the planner estimating
> multiple rows out?". But most of the time it wouldn't matter much.
> (And I think you can get cases like that anyway today.)
>
> > It's not clear to me that it would even be correct to categorize those
> > somewhat-different results as "less accurate."
>
> Estimating two rows where the correct answer is one row is clearly
> "less accurate". But I suspect you'd have to be quite unlucky to
> get such a result in practice from Simon's proposal, as long as we
> weren't super-aggressive about changing ANALYZE's snapshot a lot.
>
Seems like we're agreed we can use more than one snapshot, the only
discussion is "how many?"
The more you take the more weirdness you will see, so adopting an approach
of one-snapshot-per-row seems like the worst case for accuracy, even if it
does make analyze faster.
(If we do want to speed up ANALYZE, we should use the system block sampling
approach, but the argument against that is less independence of rows.)
Keeping the discussion on reducing the impact of bernoulli sampled analyze, I
was imagining we would do one snapshot for each block of rows with default
statistics_target, so that default behavior would be unaffected. Larger
settings would be chunked according to the default, so
stats_target=10k(max) would take a 10000/100 = 100 snapshots. That approach
allows people to vary that using an existing parameter if needed.
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/;
Mission Critical Databases
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 19:11 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 20:22 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 21:35 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-08-07 18:03 ` Robert Haas <robertmhaas@gmail.com>
2020-08-07 18:41 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 18+ messages in thread
From: Robert Haas @ 2020-08-07 18:03 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
On Thu, Aug 6, 2020 at 5:35 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > It's not clear to me that it would even be correct to categorize those
> > somewhat-different results as "less accurate."
>
> Estimating two rows where the correct answer is one row is clearly
> "less accurate" [ than estimating one row ].
That's a tautology, so I can't argue with it as far as it goes.
Thinking about it more, there are really two ways to think about an
estimated row count.
On the one hand, if you think of the row count estimate as the number
of rows that are going to pop out of a node, then it's always right to
think of a unique index as limiting the number of occurrences of a
given value to 1. But, if you think of the row count estimate as a way
of estimating the amount of work that the node has to do to produce
that output, then it isn't.
If a table has a lot of inserts and deletes, or a lot of updates,
index scans might have to do a lot of extra work chasing down index
pointers to tuples that end up being invisible to our scan. The scan
may not have any filter quals at all, and even if it does, they are
likely cheap to evaluate compared to the cost of finding a locking
buffers and checking visibility, so the dominant cost of the scan is
really based on the total number of rows that are present, not the
number that are visible. Ideally, the presence of those rows would
affect the cost estimate for the node in a way very similar to
expecting to find more rows. At the same time, it doesn't work to just
bump up the row count estimate for the node, because then you'll think
more rows will be output, which might cause poor planning decisions at
higher levels.
It doesn't seem easy to get this 100% right. Tuple visibility can
change very quickly, much faster than the inter-ANALYZE interval. And
sometimes tuples can be pruned away very quickly, too, and the index
pointers may be opportunistically removed very quickly, too.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 19:11 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 20:22 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 21:35 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-07 18:03 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
@ 2020-08-07 18:41 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 18+ messages in thread
From: Tom Lane @ 2020-08-07 18:41 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
Robert Haas <robertmhaas@gmail.com> writes:
> Thinking about it more, there are really two ways to think about an
> estimated row count.
> On the one hand, if you think of the row count estimate as the number
> of rows that are going to pop out of a node, then it's always right to
> think of a unique index as limiting the number of occurrences of a
> given value to 1. But, if you think of the row count estimate as a way
> of estimating the amount of work that the node has to do to produce
> that output, then it isn't.
The planner intends its row counts to be interpreted in the first way.
We do have a rather indirect way of accounting for the cost of scanning
dead tuples and such, which is that we scale scanning costs according
to the measured physical size of the relation. That works better for
I/O costs than it does for CPU costs, but it's not completely useless
for the latter. In any case, we'd certainly not want to increase the
scan's row count estimate for that, because that would falsely inflate
our estimate of how much work upper plan levels have to do. Whatever
happens at the scan level, the upper levels aren't going to see those
dead tuples.
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 19:11 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 20:22 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
@ 2020-08-06 21:45 ` Andres Freund <andres@anarazel.de>
2020-08-06 22:02 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 18+ messages in thread
From: Andres Freund @ 2020-08-06 21:45 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
Hi,
On 2020-08-06 16:22:23 -0400, Robert Haas wrote:
> On Thu, Aug 6, 2020 at 3:11 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > (1) Without a snapshot it's hard to make any non-bogus decisions about
> > which tuples are live and which are dead. Admittedly, with Simon's
> > proposal the final totals would be spongy anyhow, but at least the
> > individual decisions produce meaningful answers.
>
> I don't think I believe this. It's impossible to make *consistent*
> decisions, but it's not difficult to make *non-bogus* decisions.
> HeapTupleSatisfiesVacuum() and HeapTupleSatifiesUpdate() both make
> such decisions, and neither takes a snapshot argument.
Yea, I don't think that's a big problem for the main table. As I just
mentioned in an email a few minutes ago, toast is a bit of a different
topic.
In fact using conceptually like a new snapshot for each sample tuple
actually seems like it'd be somewhat of an improvement over using a
single snapshot. Given that it's a sample it's not like have very
precise expectations of the precise sample, and publishing one that
solely consists of pretty old rows by the time we're done doesn't seem
like it's a meaningful improvement. I guess there's some danger of
distinctness estimates getting worse, by seeing multiple versions of the
same tuple multiple times - but they're notoriously inaccurate already,
don't think this changes much.
> > (2) I'm pretty sure there are places in the system that assume that any
> > reader of a table is using an MVCC snapshot. For instance, didn't you
> > introduce some such assumptions along with or just after getting rid of
> > SnapshotNow for catalog scans?
>
> SnapshotSelf still exists and is still used, and IIRC, it has very
> similar semantics to the old SnapshotNow, so I don't think that we
> introduced any really general assumptions of this sort. I think the
> important part of those changes was that all the code that had
> previously used SnapshotNow to examine system catalog tuples for DDL
> purposes and catcache lookups and so forth started using an MVCC scan,
> which removed one (of many) impediments to concurrent DDL. I think the
> fact that we removed SnapshotNow outright rather than just ceasing to
> use it for that purpose was mostly so that nobody would accidentally
> reintroduce code that used it for the sorts of purposes for which it
> had been used previously, and secondarily for code cleanliness.
> There's nothing wrong with it fundamentally AFAIK.
Some preaching to the choir:
IDK, there's not really much it (along with Self, Any, ...) can safely
be used for, unless you have pretty heavyweight additional locking, or
look explicitly at exactly one tuple version. Except that it's probably
unnecessary, and that there's some disaster recovery benefits, I'd be in
favor of prohibiting most snapshot types for [sys]table scans.
I'm doubtful that using the term "snapshot" for any of these is a good
choice, and I don't think there's benefit in actually going through the
snapshot APIs. Especially not when, like *Dirty, they abuse fields
inside SnapshotData to return data that can't be returned through the
normal API. It'd probably be better to have more explicit APIs for
these, rather than going through snapshot.
> It's not clear to me that it would even be correct to categorize those
> somewhat-different results as "less accurate." Tuples that are
> invisible to a query often have performance consequences very similar
> to visible tuples, in terms of the query run time.
+1
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 19:11 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 20:22 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 21:45 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
@ 2020-08-06 22:02 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-08-07 21:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 18+ messages in thread
From: Tom Lane @ 2020-08-06 22:02 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Robert Haas <robertmhaas@gmail.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
Andres Freund <andres@anarazel.de> writes:
> In fact using conceptually like a new snapshot for each sample tuple
> actually seems like it'd be somewhat of an improvement over using a
> single snapshot.
Dunno, that feels like a fairly bad idea to me. It seems like it would
overemphasize the behavior of whatever queries happened to be running
concurrently with the ANALYZE. I do follow the argument that using a
single snapshot for the whole ANALYZE overemphasizes a single instant
in time, but I don't think that leads to the conclusion that we shouldn't
use a snapshot at all.
Another angle that would be worth considering, aside from the issue
of whether the sample used for pg_statistic becomes more or less
representative, is what impact all this would have on the tuple count
estimates that go to the stats collector and pg_class.reltuples.
Right now, we don't have a great story at all on how the stats collector's
count is affected by combining VACUUM/ANALYZE table-wide counts with
the incremental deltas reported by transactions happening concurrently
with VACUUM/ANALYZE. Would changing this behavior make that better,
or worse, or about the same?
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 19:11 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 20:22 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 21:45 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 22:02 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-08-07 21:37 ` Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 18+ messages in thread
From: Andres Freund @ 2020-08-07 21:37 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
Hi,
On 2020-08-06 18:02:26 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > In fact using conceptually like a new snapshot for each sample tuple
> > actually seems like it'd be somewhat of an improvement over using a
> > single snapshot.
>
> Dunno, that feels like a fairly bad idea to me. It seems like it would
> overemphasize the behavior of whatever queries happened to be running
> concurrently with the ANALYZE. I do follow the argument that using a
> single snapshot for the whole ANALYZE overemphasizes a single instant
> in time, but I don't think that leads to the conclusion that we shouldn't
> use a snapshot at all.
I didn't actually want to suggest that we should take a separate
snapshot for every sampled row - that'd be excessively costly. What I
wanted to say was that I don't think that I don't see a clear accuraccy
benefit. E.g. not seeing any of the values inserted more recently will
under-emphasize those in the histogram.
What precisely do you mean with "overemphasize" above? I mean those will
e the rows most likely to live after the analyze is done, so including
them doesn't seem like a bad thing to me?
> Another angle that would be worth considering, aside from the issue
> of whether the sample used for pg_statistic becomes more or less
> representative, is what impact all this would have on the tuple count
> estimates that go to the stats collector and pg_class.reltuples.
> Right now, we don't have a great story at all on how the stats collector's
> count is affected by combining VACUUM/ANALYZE table-wide counts with
> the incremental deltas reported by transactions happening concurrently
> with VACUUM/ANALYZE. Would changing this behavior make that better,
> or worse, or about the same?
Hm. Vacuum already counts rows that are inserted concurrently with the
vacuum scan, if it encounters them. Analyze doesn't. Seems like we'd at
least be wrong in a more consistent manner than before...
IIUC both analyze and vacuum will overwrite concurrent changes to
n_live_tuples. So taking concurrently committed changes into account
seems like it'd be the right thing?
We probably could make this more accurate by accounting separately for
"recently inserted and committed" rows, and taking the difference of
n_live_tuples before/after into account. But I'm a bit doubtful that
it's worth it?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-06 18:25 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Robert Haas <robertmhaas@gmail.com>
@ 2020-08-06 21:26 ` Andres Freund <andres@anarazel.de>
1 sibling, 0 replies; 18+ messages in thread
From: Andres Freund @ 2020-08-06 21:26 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alvaro Herrera <alvherre@2ndquadrant.com>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Simon Riggs <simon@2ndquadrant.com>
Hi,
On 2020-08-06 14:48:52 -0400, Robert Haas wrote:
> On Thu, Aug 6, 2020 at 2:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > +1 for removal. It's not clear to me that we'd ever put it back.
> > Long-running ANALYZE snapshots are indeed a problem, but Simon's proposal
> > upthread to just take a new one every so often seems like a much cleaner
> > and simpler answer than having onlookers assume that it's safe to ignore
> > ANALYZE processes. (Given that ANALYZE can invoke user-defined functions,
> > and can be invoked from inside user transactions, any such assumption
> > seems horribly dangerous.
>
> Not to get too far from the proposal on the table of just removing
> something that's been unused for a really long time, which stands on
> its own merits, but if a particular ANALYZE doesn't invoke any
> user-defined functions and isn't run inside a transaction, could we
> skip acquiring a snapshot altogether? That's an extremely common case,
> though by no means universal.
I don't think so, at least not in very common situations. E.g. as long
as there's a toast table we need to hold a snapshot to ensure that we
don't get failures looking up toasted datums. IIRC there were some other
similar issues that I can't quite recall right now.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
@ 2020-08-07 21:35 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-30 00:06 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Tom Lane <tgl@sss.pgh.pa.us>
2 siblings, 1 reply; 18+ messages in thread
From: Alvaro Herrera @ 2020-08-07 21:35 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; simon@2ndquadrant.com
On 2020-Aug-05, Andres Freund wrote:
> I'm mildly against that, because I'd really like to start making use of
> the flag. Not so much for cancellations, but to avoid the drastic impact
> analyze has on bloat. In OLTP workloads with big tables, and without
> disabled cost limiting for analyze (or slow IO), the snapshot that
> analyze holds is often by far the transaction with the oldest xmin.
I pushed despite the objection because it seemed that downstream
discussion was largely favorable to the change, and there's a different
proposal to solve the bloat problem for analyze; and also:
> Only mildly against because it'd not be hard to reintroduce once we need
> it.
Thanks for the discussion!
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: PROC_IN_ANALYZE stillborn 13 years ago
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Andres Freund <andres@anarazel.de>
2020-08-07 21:35 ` Re: PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2020-08-30 00:06 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 18+ messages in thread
From: Tom Lane @ 2020-08-30 00:06 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Andres Freund <andres@anarazel.de>; James Coleman <jtc331@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; simon@2ndquadrant.com
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> I pushed despite the objection because it seemed that downstream
> discussion was largely favorable to the change, and there's a different
> proposal to solve the bloat problem for analyze; and also:
Note that this quasi-related patch has pretty thoroughly hijacked
the CF entry for James' original docs patch proposal. The cfbot
thinks that that's the latest patch in the original thread, and
unsurprisingly is failing to apply it.
Since the discussion was all over the place, I'm not sure whether
there's still a live docs patch proposal or not; but if so, somebody
should repost that patch (and go back to the original thread title).
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2020-08-30 00:06 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-05 23:55 PROC_IN_ANALYZE stillborn 13 years ago Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-06 01:07 ` Andres Freund <andres@anarazel.de>
2020-08-06 08:17 ` Simon Riggs <simon@2ndquadrant.com>
2020-08-06 18:25 ` Robert Haas <robertmhaas@gmail.com>
2020-08-06 18:37 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 18:48 ` Robert Haas <robertmhaas@gmail.com>
2020-08-06 19:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 20:22 ` Robert Haas <robertmhaas@gmail.com>
2020-08-06 21:35 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-08-07 04:54 ` Simon Riggs <simon@2ndquadrant.com>
2020-08-07 18:03 ` Robert Haas <robertmhaas@gmail.com>
2020-08-07 18:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-08-06 21:45 ` Andres Freund <andres@anarazel.de>
2020-08-06 22:02 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-08-07 21:37 ` Andres Freund <andres@anarazel.de>
2020-08-06 21:26 ` Andres Freund <andres@anarazel.de>
2020-08-07 21:35 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-08-30 00:06 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox