Re: recovering from "found xmin ... from before relfrozenxid ..."

Hi,

Thanks for sharing your thoughts. Please find my comments inline below:


>
> I think here we should report that we haven't done what was asked.
> +                       /* Nothing to do if the itemid is unused or
> already dead. */
> +                       if (!ItemIdIsUsed(itemid) || ItemIdIsDead(itemid))
> +                               continue;
>
>
Okay. Will add a log message saying "skipping tid ... because ..."


> Also, should we try to fix VM along the way?
>

I think we should let VACUUM do that.


> Are there any caveats with concurrent VACUUM? (I do not see any, just
> asking)
>

As of now, I don't see any.


> It would be good to have some checks for interrupts in safe places.
>

I think I have already added those wherever I felt it was required. If you
feel it's missing somewhere, it could be good if you could point it out.


> I think we should not trust user entierly here. I'd prefer validation and
> graceful exit, not a core dump.
> +               Assert(noffs <= PageGetMaxOffsetNumber(page));
>
>
Yeah, sounds reasonable. Will do that.


> For some reason we had unlogged versions of these functions. But I do not
> recall exact rationale..
> Also, I'd be happy if we had something like "Restore this tuple iff this
> does not break unique constraint". To do so we need to sort tids by
> xmin\xmax, to revive most recent data first.
>

I didn't get this point. Could you please elaborate.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com