From: | Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> |
---|---|
To: | Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(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-26 11:36:09 |
Message-ID: | CAE9k0PmWYftk_-X0iZSG=V2H08u4awYhk2ZHh2arFG2wZ4G=6A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks for the review. Please find my comments inline below:
On Tue, Aug 25, 2020 at 5:47 PM Masahiko Sawada
<masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
>
> Let me share other comments on the latest version patch:
>
> Some words need to be tagged. For instance, I found the following words:
>
> VACUUM
> DISABLE_PAGE_SKIPPING
> HEAP_XMIN_FROZEN
> HEAP_XMAX_INVALID
>
Okay, done.
> ---
> +test=# select ctid from t1 where xmin = 507;
> + ctid
> +-------
> + (0,3)
> +(1 row)
> +
> +test=# select heap_force_freeze('t1'::regclass, ARRAY['(0, 3)']::tid[]);
> +-[ RECORD 1 ]-----+-
> +heap_force_freeze |
>
> I think it's better to use a consistent output format. The former uses
> the normal format whereas the latter uses the expanded format.
>
Yep, makes sense, done.
> ---
> + <note>
> + <para>
> + While performing surgery on a damaged relation, we must not be doing anything
> + else on that relation in parallel. This is to ensure that when we are
> + operating on a damaged tuple there is no other transaction trying to modify
> + that tuple.
> + </para>
> + </note>
>
> If we prefer to avoid concurrent operations on the target relation why
> don't we use AccessExclusiveLock?
>
Removed this note from the documentation and added a note saying: "The
user needs to ensure that they do not operate pg_force_freeze function
on a deleted tuple because it may revive the deleted tuple."
> ---
> +CREATE FUNCTION heap_force_kill(reloid regclass, tids tid[])
> +RETURNS VOID
> +AS 'MODULE_PATHNAME', 'heap_force_kill'
> +LANGUAGE C STRICT;
>
> +CREATE FUNCTION heap_force_freeze(reloid regclass, tids tid[])
> +RETURNS VOID
> +AS 'MODULE_PATHNAME', 'heap_force_freeze'
> +LANGUAGE C STRICT;
>
> I think these functions should be PARALLEL UNSAFE.
>
By default the functions are marked PARALLEL UNSAFE, so I think there
is nothing to do here.
Attached patch with above changes.
This patch also takes care of all the other review comments from - [1].
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
v8-0001-Add-contrib-pg_surgery-to-perform-surgery-on-a-damag.patch | text/x-patch | 28.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Dagfinn Ilmari Mannsåker | 2020-08-26 11:47:06 | Re: LWLockAcquire and LockBuffer mode argument |
Previous Message | Peter Eisentraut | 2020-08-26 11:14:41 | Re: use pg_get_functiondef() in pg_dump |