From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, "Wood, Dan" <hexpert(at)amazon(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, pgsql-committers(at)postgresql(dot)org, "Wong, Yi Wen" <yiwong(at)amazon(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple |
Date: | 2017-12-14 22:00:17 |
Message-ID: | 20171214220017.6dax6ne7ru4ggadr@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On 2017-12-07 18:32:51 +0900, Michael Paquier wrote:
> On Thu, Dec 7, 2017 at 5:23 AM, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> > Looking at 0002: I agree with the stuff being done here.
>
> The level of details you are providing with a proper error code is an
> improvement over the first version proposed in my opinion.
>
> > I think a
> > couple of these checks could be moved one block outerwards in term of
> > scope; I don't see any reason why the check should not apply in that
> > case. I didn't catch any place missing additional checks.
>
> In FreezeMultiXactId() wouldn't it be better to issue an error as well
> for this assertion?
> Assert(!TransactionIdPrecedes(members[i].xid, cutoff_xid));
I'm not really concerned that much about pure lockers, they don't cause
permanent data corruption...
> > Despite these being "shouldn't happen" conditions, I think we should
> > turn these up all the way to ereports with an errcode and all, and also
> > report the XIDs being complained about. No translation required,
> > though. Other than those changes and minor copy editing a commit
> > (attached), 0002 looks good to me.
If you want to go around doing that in some more places we can do so in
master only...
> + if (!(tuple->t_infomask & HEAP_XMAX_LOCK_ONLY) &&
> + TransactionIdDidCommit(xid))
> + ereport(ERROR,
> + (errcode(ERRCODE_DATA_CORRUPTED),
> + errmsg("can't freeze committed xmax %u", xid)));
> The usual wording used in errmsg is not the "can't" but "cannot".
>
> + ereport(ERROR,
> + (errcode(ERRCODE_DATA_CORRUPTED),
> + errmsg_internal("uncommitted Xmin %u from
> before xid cutoff %u needs to be frozen",
> + xid, cutoff_xid)));
> "Xmin" I have never seen, but "xmin" I did.
Changed...
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2017-12-14 22:19:39 | pgsql: Tighten configure's test for __builtin_constant_p(). |
Previous Message | Andres Freund | 2017-12-14 20:38:03 | Re: pgsql: Provide overflow safe integer math inline functions. |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2017-12-14 22:02:00 | Re: incorrect error message, while dropping PROCEDURE |
Previous Message | Michael Paquier | 2017-12-14 21:46:10 | Re: [HACKERS] Assertion failure when the non-exclusive pg_stop_backup aborted. |