On Mon, 2 Mar 2020 at 21:42, Hamid Akhtar <hamid.akhtar@gmail.com> wrote:
>
> Here is the v2 of the same patch after rebasing it and running it through pgindent. There are no other code changes.
>
Thank you for working on this. I think what this patch tries to
achieve would be helpful to inform orphaned prepared transactions that
can be cause of inefficient vacuum to users.
As far as I read the patch, the setting this feature using newly added
parameters seems to be complicated to me. IIUC, even if a prepared
transactions is enough older than max_age_prepared_xacts, we don't
warn if it doesn't elapsed prepared_xacts_vacuum_warn_timeout since
when the "first" prepared transaction is created. And the first
prepared transaction means that the first entry for
TwoPhaseStateData->prepXacts. Therefore, if there is always more than
one prepared transaction, we don't warn for at least
prepared_xacts_vacuum_warn_timeout seconds even if the first added
prepared transaction is already removed. So I'm not sure how we can
think the setting value of prepared_xacts_vacuum_warn_timeout.
Regarding the warning message, I wonder if the current message is too
detailed. If we want to inform that there is orphaned prepared
transactions to users, it seems to me that it's enough to report the
existence (and possibly the number of orphaned prepared transactions),
rather than individual details.
Given that the above things, we can simply think this feature; we can
have only max_age_prepared_xacts, and autovacuum checks the minimum of
prepared_at of prepared transactions, and compare it to
max_age_prepared_xacts. We can warn if (CurrentTimestamp -
min(prepared_at)) > max_age_prepared_xacts. In addition, if we also
want to control this behavior by the age of xid, we can have another
GUC parameter for comparing the age(min(xid of prepared transactions))
to that value.
Finally, regarding the name of parameters, when we mention the age of
transaction it means the age of xid of the transaction, not the time.
Please refer to other GUC parameter having "age" in its name such as
autovacuum_freeze_max_age, vacuum_freeze_min_age. The patch adds
max_age_prepared_xacts but I think it should be renamed. For example,
prepared_xact_warn_min_duration is for time and
prepared_xact_warn_max_age for age.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
