From: | Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com> |
---|---|
To: | Kasahara Tatsuhito <kasahara(dot)tatsuhito(at)gmail(dot)com> |
Cc: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Tid scan increments value of pg_stat_all_tables.seq_scan. (but not seq_tup_read) |
Date: | 2020-02-06 14:01:14 |
Message-ID: | CA+fd4k4LvvZxYWhORH2o3WGStxRiCX6g8_-kcrvRBDWS2POj5A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, 6 Feb 2020 at 19:12, Kasahara Tatsuhito
<kasahara(dot)tatsuhito(at)gmail(dot)com> wrote:
>
> HI,
>
> On Thu, Feb 6, 2020 at 3:24 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> > > I added a new (but minimal) isolation test for the case of tid scan.
> > > (v12 and HEAD will be failed this test. v11 and HEAD with my patch
> > > will be passed)
> >
> > Isn't this test scenario a bit overkill? We can simply test that
> > as follows, instead.
> > CREATE TABLE test_tidscan AS SELECT 1 AS id;
> > BEGIN ISOLATION LEVEL SERIALIZABLE;
> > SELECT * FROM test_tidscan WHERE ctid = '(0,1)';
> > SELECT locktype, mode FROM pg_locks WHERE pid = pg_backend_pid() AND mode = 'SIReadLock';
> > COMMIT;
> >
> > In the expected file, the result of query looking at pg_locks
> > should be matched with the following.
> >
> > locktype | mode
> > ----------+------------
> > tuple | SIReadLock
> Thanks for your reply.
> Hmm, it's an simple and might be the better way than adding isolation test.
>
> I added above test case to regress/sql/tidscan.sql.
> Attach the patch.
>
I've tested predicate locking including promotion cases with v3 patch
and it works fine.
+table_beginscan_tid(Relation rel, Snapshot snapshot,
+ int nkeys, struct ScanKeyData *key)
+{
+ uint32 flags = SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE;
IIUC setting SO_ALLOW_STRAT, SO_ALLOW_SYNC and SO_ALLOW_PAGEMODE has
no meaning during tid scan. I think v11 also should be the same.
Why did you remove SO_TYPE_TIDSCAN from the previous version patch? I
think it's better to add it and then we can set only SO_TYPE_TIDSCAN
to the scan option of tid scan.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2020-02-06 14:23:42 | Re: bad logging around broken restore_command |
Previous Message | Kyotaro Horiguchi | 2020-02-06 13:56:40 | Re: Index Skip Scan |