RE: Patch for migration of the pg_commit_ts directory - Mailing list pgsql-hackers

From Hayato Kuroda (Fujitsu)
Subject RE: Patch for migration of the pg_commit_ts directory
Date
Msg-id OS9PR01MB121493384C07100ED5AF5E1E6F568A@OS9PR01MB12149.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: Patch for migration of the pg_commit_ts directory  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: Patch for migration of the pg_commit_ts directory
List pgsql-hackers
Dear Amit,

> One of
> the use case I recall is to detect conflicts with accuracy after
> upgrade. See docs [1] ("Commit timestamps and origin data are not
> preserved during the upgrade. ..) I think this note needs an update
> after this patch.

Right, and code comments [a] should be also updated.

BTW, is there a possibility that we can export and import xmin of the conflict
slot to retain dead tuples even after the upgrade? Of course it's out-of-scope
from this thread.

> res = executeQueryOrDie(conn, "SELECT setting FROM pg_settings "
> + "WHERE name = 'track_commit_timestamp'");
> + track_commit_timestamp_on = strcmp(PQgetvalue(res, 0, 0), "on") == 0;
> 
> As this is a boolean variable, what if the user sets the value of this
> GUC as true, will the above work correctly?

Per my understanding, it is OK to use "on"/"off" notation. Below contains the
analysis.

pg_settings uses an SQL function pg_show_all_settings, and it is implemneted by
the C function show_all_settings(). And GetConfigOptionValues()->ShowGUCOption()
actualy create the output for the setting column in the view. According to the
function, the boolean would be the either "on" or "off" unless the GUC has show_hook.

```
    switch (record->vartype)
    {
        case PGC_BOOL:
            {
                const struct config_bool *conf = &record->_bool;

                if (conf->show_hook)
                    val = conf->show_hook();
                else
                    val = *conf->variable ? "on" : "off";
            }
            break;
```

It mactches my experiment below.

```
$ cat data_pub/postgresql.conf | grep track_commit_timestamp
track_commit_timestamp = true   # collect timestamp of transaction commit
$ psql -U postgres -p 5432
postgres=# SELECT setting FROM pg_settings WHERE name = 'track_commit_timestamp';
 setting 
---------
 on
(1 row)
```

> Also, _on in the variable name appears bit odd.

I have two options, thought?
 - commit_ts_is_enabled,
 - track_commit_timestmap, same as GUC variable.

[a]: https://github.com/postgres/postgres/blob/master/src/bin/pg_upgrade/pg_upgrade.c#L215

Best regards,
Hayato Kuroda
FUJITSU LIMITED


pgsql-hackers by date:

Previous
From: Ajin Cherian
Date:
Subject: Re: [PATCH] Support automatic sequence replication
Next
From: Amit Kapila
Date:
Subject: Re: Patch for migration of the pg_commit_ts directory