Re: enhance wraparound warnings - Mailing list pgsql-hackers
| From | Chao Li |
|---|---|
| Subject | Re: enhance wraparound warnings |
| Date | |
| Msg-id | C71E13D6-D919-477F-BE90-82DE832E2738@gmail.com Whole thread Raw |
| In response to | Re: enhance wraparound warnings (Nathan Bossart <nathandbossart@gmail.com>) |
| List | pgsql-hackers |
Hi Nathan,
I just reviewed the patch. My comments are mainly in 0001, and a few nits on 0003. For 0002, the code change is quite
straightforward,I am not sure the value bumping to has been discussed.
> On Dec 12, 2025, at 04:28, Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> rebased
>
> --
> nathan
>
<v2-0001-Add-percentage-of-transaction-IDs-that-are-availa.patch><v2-0002-Bump-transaction-ID-limit-to-warn-at-100M.patch><v2-0003-Perodically-emit-server-logs-when-fewer-than-500M.patch>
1 - 0001
```
+ (double) (multiWrapLimit - result) / PG_INT32_MAX * 100),
```
I don’t feel good with using PG_INT32_MAX as denominator, though the value is correct.
Looking at the code of how xidWrapLimit is calculated:
```
/*
* The place where we actually get into deep trouble is halfway around
* from the oldest potentially-existing XID. (This calculation is
* probably off by one or two counts, because the special XIDs reduce the
* size of the loop a little bit. But we throw in plenty of slop below,
* so it doesn't matter.)
*/
xidWrapLimit = oldest_datfrozenxid + (MaxTransactionId >> 1);
if (xidWrapLimit < FirstNormalTransactionId)
xidWrapLimit += FirstNormalTransactionId;
```
Where "(MaxTransactionId >> 1)” has the same value as PG_INT32_MAX. But if one day xid is changed to 64 bits, that code
doesn’tneed to updated, while these patched code will need to be updated.
So, can we define a const in transom.h like:
```
#define MaxTransactionId ((TransactionId) 0xFFFFFFFF)
#define WrapAroundWindow (MaxTransactionId>>1)
```
And use WrapAroundWindow in all places.
2 - 0001
```
+ errdetail("Approximately %.2f%% of MultiXactIds are available for use.",
```
“%.2f%%” shows only 2 digits after dot. xidWrapLimit is roughly 2B, when remaining goes down to 107374, it will shows
“0.00%”.IMO, when remaining is a large number, percentage makes more sense, while an exact number is clearer when the
numberis relatively small. So, can we show both percentage and exact number? Or shows the exact number when percentage
is0.00%?
3 - 0001
```
<programlisting>
WARNING: database "mydb" must be vacuumed within 39985967 transactions
+DETAIL: Approximately 1.86% of transactions IDs are available for use.
```
Typo: " transactions IDs” => " transaction IDs"
4 - 0003
```
Subject: [PATCH v2 3/3] Perodically emit server logs when fewer than 500M
```
Typo: Perodically => Periodically
5 - 0003
```
+ xidLogLimit = xidWrapLimit - 500000000;
```
Instead of hardcode 500M, do we want to consider autovacuum_freeze_max_age? If a deployment sets
autovacuum_freeze_max_age> 500M, then vacuum would be triggered first, then this log can get kinda non-intuitive. But
ifa vacuum cannot freeze anything tuple, then this log will still make sense. I am not sure. Maybe not a real problem.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
pgsql-hackers by date: