From: | Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru>, MBeena Emerson <mbeena(dot)emerson(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stephen Frost <sfrost(at)snowman(dot)net> |
Subject: | Re: recovering from "found xmin ... from before relfrozenxid ..." |
Date: | 2020-08-12 13:26:52 |
Message-ID: | CAE9k0Pk0Wq_CdJwrx+iKuKHiwj=R4hEL3Pf9hsAXAM+EhfwgyQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks Robert for the review. Please find my comments inline below:
On Fri, Aug 7, 2020 at 9:21 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Thu, Aug 6, 2020 at 9:23 AM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
> > Attached v4 patch fixes the latest comments from Robert and Masahiko-san.
>
> Compiler warning:
>
> heap_surgery.c:136:13: error: comparison of unsigned expression < 0 is
> always false [-Werror,-Wtautological-compare]
> if (blkno < 0 || blkno >= nblocks)
> ~~~~~ ^ ~
>
Fixed.
> There's a certain inconsistency to these messages:
>
> rhaas=# create table foo (a int);
> CREATE TABLE
> rhaas=# insert into foo values (1);
> INSERT 0 1
> rhaas=# select heap_force_kill('foo'::regclass, array['(0,2)'::tid]);
> NOTICE: skipping tid (0, 2) because it contains an invalid offset
> heap_force_kill
> -----------------
>
> (1 row)
>
> rhaas=# select heap_force_kill('foo'::regclass, array['(1,0)'::tid]);
> ERROR: invalid item pointer
> LOCATION: tids_same_page_fetch_offnums, heap_surgery.c:347
> rhaas=# select heap_force_kill('foo'::regclass, array['(1,1)'::tid]);
> ERROR: block number 1 is out of range for relation "foo"
>
> From a user perspective it seems like I've made three very similar
> mistakes: in the first case, the offset is too high, in the second
> case it's too low, and in the third case the block number is out of
> range. But in one case I get a NOTICE and in the other two cases I get
> an ERROR. In one case I get the relation name and in the other two
> cases I don't. The two complaints about an invalid offset are phrased
> completely differently from each other. For example, suppose you do
> this:
>
> ERROR: tid (%u, %u) is invalid for relation "%s" because the block
> number is out of range (%u..%u)
> ERROR: tid (%u, %u) is invalid for relation "%s" because the item
> number is out of range for this block (%u..%u)
> ERROR: tid (%u, %u) is invalid for relation "%s" because the item is unused
> ERROR: tid (%u, %u) is invalid for relation "%s" because the item is dead
>
Corrected.
> I think I misled you when I said to use pg_class_aclcheck. I think it
> should actually be pg_class_ownercheck.
>
okay, I've changed it to pg_class_ownercheck.
> I think the relkind sanity check should permit RELKIND_MATVIEW also.
>
Yeah, actually we should allow MATVIEW, don't know why I thought of
blocking it earlier.
> It's unclear to me why the freeze logic here shouldn't do this part
> what heap_prepare_freeze_tuple() does when freezing xmax:
>
> frz->t_infomask2 &= ~HEAP_HOT_UPDATED;
> frz->t_infomask2 &= ~HEAP_KEYS_UPDATED;
>
Yeah, we should have these changes when freezing the xmax.
> Likewise, why should we not freeze or invalidate xvac in the case
> where tuple->t_infomask & HEAP_MOVED, as heap_prepare_freeze_tuple()
> would do?
>
Again, we should have this as well.
Apart from above, this time I've also added the documentation on
pg_surgery module and added a few more test-cases.
Attached patch with above changes.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
v5-0001-Add-contrib-pg_surgery-to-perform-surgery-on-a-damag.patch | text/x-patch | 25.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2020-08-12 13:37:08 | Re: 回复:how to create index concurrently on partitioned table |
Previous Message | Andy Fan | 2020-08-12 13:06:31 | Re: Allows Extend Protocol support CURSOR_OPT_HOLD with prepared stmt. |