Thread: Small TRUNCATE glitch
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
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/ > Please line up in a tree and maintain the heap invariant while > boarding. Thank you for flying nlogn airlines.
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
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
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
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. +
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. 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? -- Regards, Alex > --------------------------------------------------------------------------- > > 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
Attachment
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
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
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
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. +
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