Hi,

> @@ -4032,6 +4039,24 @@ GlobalVisTestShouldUpdate(GlobalVisState *state)
>  static void
>  GlobalVisUpdateApply(ComputeXidHorizonsResult *horizons)
>  {
> +     /* assert non-decreasing nature of horizons */
> +     Assert(FullTransactionIdFollowsOrEquals(
> +                        FullXidRelativeTo(horizons->latest_completed,
> +                                                              
> horizons->shared_oldest_nonremovable),
> +                        GlobalVisSharedRels.maybe_needed));
> +     Assert(FullTransactionIdFollowsOrEquals(
> +                        FullXidRelativeTo(horizons->latest_completed,
> +                                                              
> horizons->catalog_oldest_nonremovable),
> +                        GlobalVisCatalogRels.maybe_needed));
> +     Assert(FullTransactionIdFollowsOrEquals(
> +                        FullXidRelativeTo(horizons->latest_completed,
> +                                                              
> horizons->data_oldest_nonremovable),
> +                        GlobalVisDataRels.maybe_needed));
> +     Assert(FullTransactionIdFollowsOrEquals(
> +                        FullXidRelativeTo(horizons->latest_completed,
> +                                                              
> horizons->temp_oldest_nonremovable),
> +                        GlobalVisTempRels.maybe_needed));
> +
>       GlobalVisSharedRels.maybe_needed =
>               FullXidRelativeTo(horizons->latest_completed,
>                                                 
> horizons->shared_oldest_nonremovable);

Thinking more about it, I don't think these are correct. See the
following comment in procarray.c:

 * Note: despite the above, it's possible for the calculated values to move
 * backwards on repeated calls. The calculated values are conservative, so
 * that anything older is definitely not considered as running by anyone
 * anymore, but the exact values calculated depend on a number of things. For
 * example, if there are no transactions running in the current database, the
 * horizon for normal tables will be latestCompletedXid. If a transaction
 * begins after that, its xmin will include in-progress transactions in other
 * databases that started earlier, so another call will return a lower value.
 * Nonetheless it is safe to vacuum a table in the current database with the
 * first result.  There are also replication-related effects: a walsender
 * process can set its xmin based on transactions that are no longer running
 * on the primary but are still being replayed on the standby, thus possibly
 * making the values go backwards.  In this case there is a possibility that
 * we lose data that the standby would like to have, but unless the standby
 * uses a replication slot to make its xmin persistent there is little we can
 * do about that --- data is only protected if the walsender runs continuously
 * while queries are executed on the standby.  (The Hot Standby code deals
 * with such cases by failing standby queries that needed to access
 * already-removed data, so there's no integrity bug.)  The computed values
 * are also adjusted with vacuum_defer_cleanup_age, so increasing that setting
 * on the fly is another easy way to make horizons move backwards, with no
 * consequences for data integrity.

Greetings,

Andres Freund


Reply via email to