Re: visibility map corruption

Hi,

On 7/7/21 3:49 AM, Bruce Momjian wrote:
> On Tue, Jul  6, 2021 at 08:36:13PM -0400, Bruce Momjian wrote:
>> On Tue, Jul  6, 2021 at 06:49:10PM -0400, Bruce Momjian wrote:
>>> My point is that there are a lot internals involved here that are not
>>> part of pg_upgrade, though it probably only affects pg_upgrade.  Anyway,
>>> Bertrand patch seems to have what I need.
>> One question is how do we want to handle cases where -x next_xid is used
>> but -u oldestXid is not used?  Compute a value for oldestXid like we did
>> previously?  Throw an error?  Leave oldestXid unchanged?  I am thinking
>> the last option.
> Here is a modified version of Bertrand's patch, with docs, that does the
> last option.

Thanks for having looked at it.

It looks good to me, but i have one question:

+    printf(_("  -u, --oldest-transaction-id=XID  set oldest transaction 
ID\n"));

and

+                   if (!TransactionIdIsNormal(set_oldest_xid))
+                   {
+                        pg_log_error("oldest transaction ID (-u) must 
be greater or equal to %u", FirstNormalTransactionId);
+                        exit(1);
+                   }

I am wondering if we should not keep my original proposal "oldest 
unfrozen transaction" (as compare to "oldest transaction") in both 
output to:

- make the wording similar with what we can found in StartupXLOG():

     ereport(DEBUG1,
             (errmsg_internal("oldest unfrozen transaction ID: %u, in 
database %u",
                              checkPoint.oldestXid, 
checkPoint.oldestXidDB)));

- give the new  "-u" a sense (somehow) from a naming point of view.

What do you think?

Thanks

Bertrand