From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, Kevin Grittner <kgrittn(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: fixing old_snapshot_threshold's time->xid mapping |
Date: | 2020-04-22 05:39:20 |
Message-ID: | CAFiTN-trHVGY_gRFGm9-vVYeQOjxQ-YQG1ngyLCqAvD=3+DoLQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Apr 21, 2020 at 4:52 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Tue, Apr 21, 2020 at 3:44 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> >
> > On Tue, Apr 21, 2020 at 2:05 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> > > As before, these two apply on top of Robert's patches (or at least his
> > > 0001 and 0002).
> >
> > While trying to figure out if Robert's 0003 patch was correct, I added
> > yet another patch to this stack to test it. 0006 does basic xid map
> > maintenance that exercises the cases 0003 fixes, and I think it
> > demonstrates that they now work correctly.
>
> +1, I think we should also add a way to test the case, where we
> advance the timestamp by multiple slots. I see that you have such
> case
> e.g
> +# test adding minutes while the map is not full
> +set_time('3000-01-01 02:01:00Z');
> +is(summarize_mapping(), "2|02:00:00|02:01:00");
> +set_time('3000-01-01 02:05:00Z');
> +is(summarize_mapping(), "6|02:00:00|02:05:00");
> +set_time('3000-01-01 02:19:00Z');
> +is(summarize_mapping(), "20|02:00:00|02:19:00");
>
> But, I think we should try to extend it to test that we have put the
> new xid only in those slots where we suppose to and not in other
> slots?.
I feel that we should. probably fix this check as well? Because if ts
> update_ts then it will go to else part then there it will finally
end up in the last slot only so I think we can use this case also as
fast exit.
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 93a0c04..644d9b1 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -1831,7 +1831,7 @@
TransactionIdLimitedForOldSnapshots(TransactionId recentXmin,
if (!same_ts_as_threshold)
{
- if (ts == update_ts)
+ if (ts >= update_ts)
{
xlimit = latest_xmin;
if (NormalTransactionIdFollows(xlimit,
recentXmin))
This patch can be applied on top of other v5 patches.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
v5-0007-Fix-check-while-computing-transaction-xid-limit.patch | application/octet-stream | 791 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | tushar | 2020-04-22 06:33:01 | [IBM z Systems] Rpm package issues. |
Previous Message | Alexander Lakhin | 2020-04-22 04:00:00 | Re: HEAPDEBUGALL is broken |