oldest/newestCommitTs output by pg_controldata

Started by Joe Conwayover 10 years ago13 messageshackers
Jump to latest
#1Joe Conway
mail@joeconway.com

In looking at the exposing pg_controldata as function patch again, it
struck me that the following output seems wrong:

--------------
Latest checkpoint's oldestCommitTs: 20257
Latest checkpoint's newestCommitTs: 84159
--------------

Those numbers are XIDs, not timestamps. Shouldn't we either emit the
actual timestamps, or else rename those to oldest/newestCommitXID?

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

#2Michael Paquier
michael@paquier.xyz
In reply to: Joe Conway (#1)
Re: oldest/newestCommitTs output by pg_controldata

On Sun, Dec 27, 2015 at 10:08 AM, Joe Conway <mail@joeconway.com> wrote:

In looking at the exposing pg_controldata as function patch again, it
struck me that the following output seems wrong:

--------------
Latest checkpoint's oldestCommitTs: 20257
Latest checkpoint's newestCommitTs: 84159
--------------

Those numbers are XIDs, not timestamps. Shouldn't we either emit the
actual timestamps, or else rename those to oldest/newestCommitXID?

I recall from the commit_ts thread that Alvaro had some real need to
make those fields XIDs and not timestamps, so +1 for renaming that as
suggested.
--
Michael

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

#3Joe Conway
mail@joeconway.com
In reply to: Michael Paquier (#2)
Re: oldest/newestCommitTs output by pg_controldata

On 12/26/2015 06:32 PM, Michael Paquier wrote:

On Sun, Dec 27, 2015 at 10:08 AM, Joe Conway <mail@joeconway.com> wrote:

In looking at the exposing pg_controldata as function patch again, it
struck me that the following output seems wrong:

--------------
Latest checkpoint's oldestCommitTs: 20257
Latest checkpoint's newestCommitTs: 84159
--------------

Those numbers are XIDs, not timestamps. Shouldn't we either emit the
actual timestamps, or else rename those to oldest/newestCommitXID?

I recall from the commit_ts thread that Alvaro had some real need to
make those fields XIDs and not timestamps, so +1 for renaming that as
suggested.

Ok, but now next question -- should we just change the user visible
output to oldestCommitXID/newestCommitXID, or should we change the
variable name everywhere it appears in source as well? Looks like each
one appears about 25-30 times scattered across 9 or 10 files. Since they
are new in 9.5, if we're going to change them, I'd think we ought to do
it now or never.

If there is consensus to make the change either way (output-only or
globally), I'll come up with a patch ASAP.

Opinions?

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joe Conway (#3)
Re: oldest/newestCommitTs output by pg_controldata

Joe Conway <mail@joeconway.com> writes:

Ok, but now next question -- should we just change the user visible
output to oldestCommitXID/newestCommitXID, or should we change the
variable name everywhere it appears in source as well? Looks like each
one appears about 25-30 times scattered across 9 or 10 files. Since they
are new in 9.5, if we're going to change them, I'd think we ought to do
it now or never.

The name is just as misleading at the source-code level, maybe more so
since they're all just numbers in C. +1 for changing it everywhere
before somebody makes a mistake based on the incorrect names.

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

#5Joe Conway
mail@joeconway.com
In reply to: Tom Lane (#4)
Re: oldest/newestCommitTs output by pg_controldata

On 12/28/2015 09:03 AM, Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

Ok, but now next question -- should we just change the user visible
output to oldestCommitXID/newestCommitXID, or should we change the
variable name everywhere it appears in source as well? Looks like each
one appears about 25-30 times scattered across 9 or 10 files. Since they
are new in 9.5, if we're going to change them, I'd think we ought to do
it now or never.

The name is just as misleading at the source-code level, maybe more so
since they're all just numbers in C. +1 for changing it everywhere
before somebody makes a mistake based on the incorrect names.

Ok, I'm on it now

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

#6Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Joe Conway (#5)
Re: oldest/newestCommitTs output by pg_controldata

Joe Conway wrote:

On 12/28/2015 09:03 AM, Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

Ok, but now next question -- should we just change the user visible
output to oldestCommitXID/newestCommitXID, or should we change the
variable name everywhere it appears in source as well? Looks like each
one appears about 25-30 times scattered across 9 or 10 files. Since they
are new in 9.5, if we're going to change them, I'd think we ought to do
it now or never.

The name is just as misleading at the source-code level, maybe more so
since they're all just numbers in C. +1 for changing it everywhere
before somebody makes a mistake based on the incorrect names.

Ok, I'm on it now

Great, thanks.

--
�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

#7Joe Conway
mail@joeconway.com
In reply to: Alvaro Herrera (#6)
Re: oldest/newestCommitTs output by pg_controldata

On 12/28/2015 09:53 AM, Alvaro Herrera wrote:

The name is just as misleading at the source-code level, maybe more so
since they're all just numbers in C. +1 for changing it everywhere
before somebody makes a mistake based on the incorrect names.

Ok, I'm on it now

Great, thanks.

I think the attached does the job. Note I settled on
(new|old)estCommitTsXid as that seemed most descriptive and not horribly
longer than before. It did mean, however, that I needed to add a single
space to all the output in pg_resetxlog and pg_controldata, which added
a fair amount to the patch size.

It is all fairly straightforward, but given the impending 9.5 release,
it'd be nice if someone has a chance to give this a quick review before
I commit.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

Attachments:

committsxid.00.difftext/x-diff; name=committsxid.00.diffDownload+286-286
#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joe Conway (#7)
Re: oldest/newestCommitTs output by pg_controldata

Joe Conway <mail@joeconway.com> writes:

I think the attached does the job. Note I settled on
(new|old)estCommitTsXid as that seemed most descriptive and not horribly
longer than before. It did mean, however, that I needed to add a single
space to all the output in pg_resetxlog and pg_controldata, which added
a fair amount to the patch size.

Hm. That will break all the translations for those messages, no?
Not sure it's worth it.

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

#9Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#8)
Re: oldest/newestCommitTs output by pg_controldata

Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

I think the attached does the job. Note I settled on
(new|old)estCommitTsXid as that seemed most descriptive and not horribly
longer than before. It did mean, however, that I needed to add a single
space to all the output in pg_resetxlog and pg_controldata, which added
a fair amount to the patch size.

Hm. That will break all the translations for those messages, no?
Not sure it's worth it.

Yeah, I would leave the others alone and just let this line have the
value shifted one more column to the right.

--
�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

#10Joe Conway
mail@joeconway.com
In reply to: Alvaro Herrera (#9)
Re: oldest/newestCommitTs output by pg_controldata

On 12/28/2015 10:30 AM, Alvaro Herrera wrote:

Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

I think the attached does the job. Note I settled on
(new|old)estCommitTsXid as that seemed most descriptive and not horribly
longer than before. It did mean, however, that I needed to add a single
space to all the output in pg_resetxlog and pg_controldata, which added
a fair amount to the patch size.

Hm. That will break all the translations for those messages, no?
Not sure it's worth it.

Yeah, I would leave the others alone and just let this line have the
value shifted one more column to the right.

Seems like the translation issue would be mostly 9.5. Maybe we should
add the space in master but not 9.5?

An alternative would be to not have a space at all for those two, e.g.

"Latest checkpoint's oldestCommitTsXid:%u\n"
"Latest checkpoint's newestCommitTsXid:%u\n"

That isn't too bad and would leave everything aligned.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

#11Joe Conway
mail@joeconway.com
In reply to: Joe Conway (#10)
Re: oldest/newestCommitTs output by pg_controldata

On 12/28/2015 10:35 AM, Joe Conway wrote:

An alternative would be to not have a space at all for those two, e.g.

"Latest checkpoint's oldestCommitTsXid:%u\n"
"Latest checkpoint's newestCommitTsXid:%u\n"

That isn't too bad and would leave everything aligned.

That seems like the best solution to me.

8<-----------------------
...
Latest checkpoint's oldestMultiXid: 1
Latest checkpoint's oldestMulti's DB: 1
Latest checkpoint's oldestCommitTsXid:0
Latest checkpoint's newestCommitTsXid:0
Time of latest checkpoint: Thu 24 Dec 2015 08:55:27 AM PST
Fake LSN counter for unlogged rels: 0/1
...
8<-----------------------

Anyone parsing based on fixed length would be ok, and so would anyone
splitting on the colon.

I retract my earlier suggestion of doing HEAD different from
REL9_5_STABLE, at least for the moment. My patch for pg_controldata
related functions is going to impact all this anyway, so we might as
well not fuss about it now.

Any objections to the attached?

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

Attachments:

committsxid-95.00.difftext/x-diff; name=committsxid-95.00.diffDownload+136-136
#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joe Conway (#11)
Re: oldest/newestCommitTs output by pg_controldata

Joe Conway <mail@joeconway.com> writes:

I retract my earlier suggestion of doing HEAD different from
REL9_5_STABLE, at least for the moment. My patch for pg_controldata
related functions is going to impact all this anyway, so we might as
well not fuss about it now.

Seems reasonable.

Any objections to the attached?

Looks OK in a quick once-over.

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

#13Joe Conway
mail@joeconway.com
In reply to: Tom Lane (#12)
Re: oldest/newestCommitTs output by pg_controldata

On 12/28/2015 11:48 AM, Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

I retract my earlier suggestion of doing HEAD different from
REL9_5_STABLE, at least for the moment. My patch for pg_controldata
related functions is going to impact all this anyway, so we might as
well not fuss about it now.

Seems reasonable.

Any objections to the attached?

Looks OK in a quick once-over.

Pushed to HEAD and 9.5

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development