Re: wal_dump output on CREATE DATABASE - Mailing list pgsql-hackers
From | Jean-Christophe Arnu |
---|---|
Subject | Re: wal_dump output on CREATE DATABASE |
Date | |
Msg-id | CAHZmTm07PA8XpL3bW4bnNj3=XzDZ0y1=-nnfirBQcQ7=2cAvZA@mail.gmail.com Whole thread Raw |
In response to | Re: wal_dump output on CREATE DATABASE (Robert Haas <robertmhaas@gmail.com>) |
Responses |
Re: wal_dump output on CREATE DATABASE
Re: wal_dump output on CREATE DATABASE |
List | pgsql-hackers |
Le jeu. 15 nov. 2018 à 19:44, Robert Haas <robertmhaas@gmail.com> a écrit :
On Tue, Nov 13, 2018 at 3:40 PM Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
> People reading pg_waldump output quickly learn to read the A/B/C format
> and what those fields mean. Breaking that into ts=A db=B relfilenode=C
> does not make that particularly clearer or easier to read. I'd say it'd
> also makes it harder to parse, and it increases the size of the output
> (both in terms of line length and data size).
I agree.
First, thank you all for your reviews.
I also agree that the A/B/C format is right (and it may be a good thing to document it, maybe by adding some changes in the doc/src/sgml/ref/pg_waldump.sgml file to this patch).
To reply to Andres, I agree we should not change things for a target format that would not fit clearly defined syntax. In that way, I agree with Tomas on the fact that people reading
pg_waldump output are quickly familiar with the A/B/C notation.
My first use case was to decode the ids with a processing script to identify each id in A/B/C or pg_waldump output with a "human readable" item. For this, my processing script connects the cluster and tries resolve the ids with simple queries (and building a local cache for this). Then it replaces each looked up id item with its corresponding text.
In some cases, this could be useful for DBA to find more easily when a specific relation was modified (searching for DELETE BTW). But that's only my use case and my little script.
Going back to the code :
As I can figure by crawling the source tree (and discovering it) there are messages with :
* A/B/C notation which seems to be the one we should adopt ( meaning ts/db/refilenode )
some are only
* A/B for the COPY message we discussed later
On the other hand, and I don't know if it's relevant, I've pointed some examples such as XLOG_RELMAP_UPDATE in relmapdesc.c which could benefit of that "notation" :
appendStringInfo(buf, "database %u tablespace %u size %u",
xlrec->dbid, xlrec->tsid, xlrec->nbytes);
appendStringInfo(buf, "database %u tablespace %u size %u",
xlrec->dbid, xlrec->tsid, xlrec->nbytes);
could be written like this :
appendStringInfo(buf, "%u/%u size %u",
xlrec->tsid, xlrec->dbid, xlrec->nbytes);
appendStringInfo(buf, "%u/%u size %u",
xlrec->tsid, xlrec->dbid, xlrec->nbytes);
In that case ts and db should also be switched. In that case the message would only by B/C which is confusing, but we have other place where "base/" is put in prefix.
The same transform may be also applied to standbydesc.c in standby_desc() function.
appendStringInfo(buf, "xid %u db %u rel %u ",
xlrec->locks[i].xid, xlrec->locks[i].dbOid,
xlrec->locks[i].relOid);
appendStringInfo(buf, "xid %u db %u rel %u ",
xlrec->locks[i].xid, xlrec->locks[i].dbOid,
xlrec->locks[i].relOid);
may be changed to
appendStringInfo(buf, "xid %u (db/rel) %u/%u ",
xlrec->locks[i].xid, xlrec->locks[i].dbOid,
xlrec->locks[i].relOid);
appendStringInfo(buf, "xid %u (db/rel) %u/%u ",
xlrec->locks[i].xid, xlrec->locks[i].dbOid,
xlrec->locks[i].relOid);
As I said, I don't know whether it's relevant to perform these changes or not.
If the A/B/C notation is to be generalized, it would be worth document it in the SGML file.
If not, the first patch provided should be enough.
Regards
--
Jean-Christophe Arnu
pgsql-hackers by date: