From: | Antonin Houska <ah(at)cybertec(dot)at> |
---|---|
To: | Michail Nikolaev <michail(dot)nikolaev(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] Full support for index LP_DEAD hint bits on standby |
Date: | 2021-11-09 11:01:44 |
Message-ID: | 42074.1636455704@antos |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Michail Nikolaev <michail(dot)nikolaev(at)gmail(dot)com> wrote:
> > I understand that the RR snapshot is used to check the MVCC behaviour, however
> > this comment seems to indicate that the RR snapshot should also prevent the
> > standb from setting the hint bits.
> > # Make sure previous queries not set the hints on standby because
> > # of RR snapshot
> > I can imagine that on the primary, but I don't think that the backend that
> > checks visibility on standby does checks other snapshots/backends. And it
> > didn't work when I ran the test manually, although I could have missed
> > something.
>
> Yes, it checks - you could see ComputeXidHorizons for details. It is
> the main part of the correctness of the whole feature. I added some
> details about it to the test.
Ah, ok. I thought that only KnownAssignedXids is used on standby, but that
would ignore the RR snapshot. It wasn't clear to me when the xmin of the
hot-standby backends is set, now I think it's done by GetSnapshotData().
> > * I can see no test for the INDEX_LP_DEAD_OK_MIN_LSN value of the
> > IndexLpDeadAllowedResult enumeration. Shouldn't there be only two values,
> > e.g. INDEX_LP_DEAD_OK and INDEX_LP_DEAD_MAYBE_OK ? Or a boolean variable (in
> > index_fetch_heap()) of the appropriate name, e.g. kill_maybe_allowed, and
> > rename the function is_index_lp_dead_allowed() to
> > is_index_lp_dead_maybe_allowed()?
>
> Yes, this way it is looks better. Done. Also, I have added some checks
> for “maybe” LSN-related logic to the test.
Attached is a proposal for a minor addition that would make sense to me, add
it if you think it's appropriate.
I think I've said enough, changing the status to "ready for committer" :-)
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachment | Content-Type | Size |
---|---|---|
test_proposal.patch | text/x-diff | 919 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2021-11-09 11:15:36 | Re: Printing backtrace of postgres processes |
Previous Message | Amit Kapila | 2021-11-09 10:03:50 | Re: Skipping logical replication transactions on subscriber side |