Small TRUNCATE glitch

Started by Tom Laneabout 18 years ago12 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

Just noticed that TRUNCATE fails to clear the stats collector's counts
for the table. I am not sure if it should reset the event counts or
not (any thoughts?) but surely it is wrong to not zero the live/dead
tuple counts.

regards, tom lane

#2Martijn van Oosterhout
kleptog@svana.org
In reply to: Tom Lane (#1)
Re: Small TRUNCATE glitch

On Thu, Apr 03, 2008 at 11:58:11AM -0400, Tom Lane wrote:

Just noticed that TRUNCATE fails to clear the stats collector's counts
for the table. I am not sure if it should reset the event counts or
not (any thoughts?) but surely it is wrong to not zero the live/dead
tuple counts.

Wern't there complaints from people regularly truncating and
refilling tables getting bad plans because they lost the statistics?

Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/

Show quoted text

Please line up in a tree and maintain the heap invariant while
boarding. Thank you for flying nlogn airlines.

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Martijn van Oosterhout (#2)
Re: Small TRUNCATE glitch

Martijn van Oosterhout <kleptog@svana.org> writes:

On Thu, Apr 03, 2008 at 11:58:11AM -0400, Tom Lane wrote:

Just noticed that TRUNCATE fails to clear the stats collector's counts
for the table. I am not sure if it should reset the event counts or
not (any thoughts?) but surely it is wrong to not zero the live/dead
tuple counts.

Wern't there complaints from people regularly truncating and
refilling tables getting bad plans because they lost the statistics?

Not related --- the planner doesn't look at pgstats data.

regards, tom lane

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#1)
Re: Small TRUNCATE glitch

Tom Lane wrote:

Just noticed that TRUNCATE fails to clear the stats collector's counts
for the table. I am not sure if it should reset the event counts or
not (any thoughts?) but surely it is wrong to not zero the live/dead
tuple counts.

Agreed, the live/dead counters should be reset. Regarding event counts,
my take is that we should have a separate statement count for truncate
(obviously not a tuple count), and the others should be left alone.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#4)
Re: Small TRUNCATE glitch

Alvaro Herrera <alvherre@commandprompt.com> writes:

Tom Lane wrote:

Just noticed that TRUNCATE fails to clear the stats collector's counts
for the table. I am not sure if it should reset the event counts or
not (any thoughts?) but surely it is wrong to not zero the live/dead
tuple counts.

Agreed, the live/dead counters should be reset. Regarding event counts,
my take is that we should have a separate statement count for truncate
(obviously not a tuple count), and the others should be left alone.

I thought some more about how to do it, and stumbled over how to cope
with TRUNCATE being rolled back. That nixed my first idea of just
having TRUNCATE send a zero-the-counters-now message.

regards, tom lane

#6Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#5)
Re: Small TRUNCATE glitch

Added to TODO:

o Clear table counters on TRUNCATE

http://archives.postgresql.org/pgsql-hackers/2008-04/msg00169.php

---------------------------------------------------------------------------

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Tom Lane wrote:

Just noticed that TRUNCATE fails to clear the stats collector's counts
for the table. I am not sure if it should reset the event counts or
not (any thoughts?) but surely it is wrong to not zero the live/dead
tuple counts.

Agreed, the live/dead counters should be reset. Regarding event counts,
my take is that we should have a separate statement count for truncate
(obviously not a tuple count), and the others should be left alone.

I thought some more about how to do it, and stumbled over how to cope
with TRUNCATE being rolled back. That nixed my first idea of just
having TRUNCATE send a zero-the-counters-now message.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#7Alex Shulgin
ash@commandprompt.com
In reply to: Bruce Momjian (#6)
Re: Small TRUNCATE glitch

Bruce Momjian <bruce@momjian.us> writes:

Added to TODO:

o Clear table counters on TRUNCATE

http://archives.postgresql.org/pgsql-hackers/2008-04/msg00169.php

Hello,

Attached is a WIP patch for this TODO.

Attachments:

0001-WIP-track-TRUNCATEs-in-pgstat-transaction-stats.patchtext/x-diffDownload+75-34
#8Alex Shulgin
ash@commandprompt.com
In reply to: Alex Shulgin (#7)
Re: Small TRUNCATE glitch

Alex Shulgin <ash@commandprompt.com> writes:

Bruce Momjian <bruce@momjian.us> writes:

Added to TODO:

o Clear table counters on TRUNCATE

http://archives.postgresql.org/pgsql-hackers/2008-04/msg00169.php

Hello,

Attached is a WIP patch for this TODO.

This part went as an attachment, which wasn't my intent:
========================================================

It does the trick by tracking if a TRUNCATE command was issued under a
(sub)transaction and uses this knowledge to reset the live/dead tuple
counters later if the transaction was committed. Testing in simple
cases shows that this clears the counters correctly, including use of
savepoints.

The 2PC part requires extending bool flag to fit the trunc flag, is this
approach sane? Given that 2PC transaction should survive server
restart, it's reasonable to expect it to also survive the upgrade, so I
see no clean way of adding another bool field to the
TwoPhasePgStatRecord struct (unless some would like to add checks on
record length, etc.).

I'm going to add some regression tests, but not sure what would be the
best location for this. The truncate.sql seems like natural choice, but
stats are not updating realtime, so I'd need to borrow some tricks from
stats.sql or better put the new tests in the stats.sql itself?

--
Alex

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alex Shulgin (#8)
Re: Small TRUNCATE glitch

Alex Shulgin wrote:

The 2PC part requires extending bool flag to fit the trunc flag, is this
approach sane? Given that 2PC transaction should survive server
restart, it's reasonable to expect it to also survive the upgrade, so I
see no clean way of adding another bool field to the
TwoPhasePgStatRecord struct (unless some would like to add checks on
record length, etc.).

I don't think we need to have 2PC files survive a pg_upgrade. It seems
perfectly okay to remove them from the new cluster at some appropriate
time, *if* they are copied from the old cluster at all (I don't think
they should be.)

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Alvaro Herrera (#9)
Re: Small TRUNCATE glitch

On 12/10/2014 03:04 AM, Alvaro Herrera wrote:

Alex Shulgin wrote:

The 2PC part requires extending bool flag to fit the trunc flag, is this
approach sane? Given that 2PC transaction should survive server
restart, it's reasonable to expect it to also survive the upgrade, so I
see no clean way of adding another bool field to the
TwoPhasePgStatRecord struct (unless some would like to add checks on
record length, etc.).

I don't think we need to have 2PC files survive a pg_upgrade. It seems
perfectly okay to remove them from the new cluster at some appropriate
time, *if* they are copied from the old cluster at all (I don't think
they should be.)

I think pg_upgrade should check if there are any prepared transactions
pending, and refuse to upgrade if there are. It could be made to work,
but it's really not worth the trouble. If there are any pending prepared
transactions in the system when you run pg_upgrade, it's more likely to
be a mistake or oversight in the first place, than on purpose.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Bruce Momjian
bruce@momjian.us
In reply to: Heikki Linnakangas (#10)
Re: Small TRUNCATE glitch

On Wed, Dec 10, 2014 at 10:32:42AM +0200, Heikki Linnakangas wrote:

I don't think we need to have 2PC files survive a pg_upgrade. It seems
perfectly okay to remove them from the new cluster at some appropriate
time, *if* they are copied from the old cluster at all (I don't think
they should be.)

I think pg_upgrade should check if there are any prepared
transactions pending, and refuse to upgrade if there are. It could
be made to work, but it's really not worth the trouble. If there are
any pending prepared transactions in the system when you run
pg_upgrade, it's more likely to be a mistake or oversight in the
first place, than on purpose.

pg_upgrade already checks for prepared transactions and errors out if
they exist; see check_for_prepared_transactions().

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Alex Shulgin
ash@commandprompt.com
In reply to: Bruce Momjian (#11)
Re: Small TRUNCATE glitch

Bruce Momjian <bruce@momjian.us> writes:

On Wed, Dec 10, 2014 at 10:32:42AM +0200, Heikki Linnakangas wrote:

I don't think we need to have 2PC files survive a pg_upgrade. It seems
perfectly okay to remove them from the new cluster at some appropriate
time, *if* they are copied from the old cluster at all (I don't think
they should be.)

I think pg_upgrade should check if there are any prepared
transactions pending, and refuse to upgrade if there are. It could
be made to work, but it's really not worth the trouble. If there are
any pending prepared transactions in the system when you run
pg_upgrade, it's more likely to be a mistake or oversight in the
first place, than on purpose.

pg_upgrade already checks for prepared transactions and errors out if
they exist; see check_for_prepared_transactions().

Alright, that's good to know. So I'm just adding a new bool field to
the 2PC pgstat record instead of the bit magic.

Attached is v0.2, now with a regression test included.

--
Alex

Attachments:

truncate-and-pgstat-v0.2.patchtext/x-diffDownload+291-18