Re: Implementing Incremental View Maintenance

Hello huyajun,

I'm sorry for delay in my response.

On Tue, 26 Jul 2022 12:00:26 +0800
huyajun <hu_yajun(at)qq(dot)com> wrote:

> I read your patch and think this processing is greet, but there is a risk of deadlock.
> Although I have not thought of a suitable processing method for the time being,
> it is also acceptable for truncate scenarios.The deadlock scene is as follows:
>
> Mv define is: select * from base_a,base_b;
> S1: truncate base_a; ― only AccessExclusiveLock base_a and not run into after trigger
> S2: insert into base_b; ― The update has been completed and the incremental refresh is started in the after trigger，RowExclusive on base_b and ExclusiveLock on mv
> S1: continue truncate mv, wait for AccessExclusiveLock on mv, wait for S2
> S2: continue refresh mv, wait for AccessShardLock on base_a, wait for S1
> So deadlock occurred

Hmm, this deadlock scenario is possible, indeed.

One idea to resolve it is to acquire RowExclusive locks on all base tables
in the BEFORE trigger. If so, S2 can not progress its process because it
waits for a RowExclusive lock on base_b, and it can not acquire ExeclusiveLock
on mv before S1 finishes.

> I also found some new issues that I would like to discuss with you

Thank you so much for your massive bug reports!

> 1. Concurrent DML causes imv data error, case like below
> Setup:
> Create table t( a int);
> Insert into t select 1 from generate_series(1,3);
> create incremental materialized view s as select count(*) from t;
>
> S1: begin;delete from t where ctid in (select ctid from t limit 1);
> S2: begin;delete from t where ctid in (select ctid from t limit 1 offset 1);
> S1: commit;
> S2: commit;
>
> After this, The count data of s becomes 2 but correct data is 1.
> I found out that the problem is probably because to our use of ctid update
> Consider user behavior unrelated to imv：
>
> Create table t( a int);
> Insert into t select 1;
> s1: BEGIN
> s1: update t set a = 2 where ctid in (select ctid from t); -- UPDATE 1
> s2: BEGIN
> s2: update t set a = 3 where ctid in (select ctid from t); -- wait row lock
> s1: COMMIT
> s2: -- UPDATE 0 -- ctid change so can't UPDATE one rows
> So we lost the s2 update
>
> 2. Sometimes it will crash when the columns of the created materialized view do not match
> Create table t( a int);
> create incremental materialized view s(z) as select sum(1) as a, sum(1) as b from t;
>
> The problem should be that colNames in rewriteQueryForIMMV does not consider this situation
>
> 3. Sometimes no error when the columns of the created materialized view do not match
> Create table t( a int);
> create incremental materialized view s(y,z) as select count(1) as b from t;
>
> But the hidden column of IMV is overwritten to z which will cause refresh failed.
>
> The problem should be that checkRuleResultList we should only skip imv hidden columns check
>
> 4. A unique index should not be created in the case of a Cartesian product
>
> create table base_a (i int primary key, j varchar);
> create table base_b (i int primary key, k varchar);
> INSERT INTO base_a VALUES
> (1,10),
> (2,20),
> (3,30),
> (4,40),
> (5,50);
> INSERT INTO base_b VALUES
> (1,101),
> (2,102),
> (3,103),
> (4,104);
> CREATE incremental MATERIALIZED VIEW s as
> select base_a.i,base_a.j from base_a,base_b; ― create error because of unique index

I am working on above issues (#1-#4) now, and I'll respond on each later.

> 5. Besides, I would like to ask you if you have considered implementing an IMV with delayed refresh?
> The advantage of delayed refresh is that it will not have much impact on write performance

Yes, I've been thinking to implement deferred maintenance since the beginning of
this IVM project. However, we've decided to start from immediate maintenance, and
will plan to propose deferred maintenance to the core after the current patch is
accepted. (I plan to implement this feature in pg_ivm extension module first,
though.)

> I probably have some ideas about it now, do you think it works?
> 1. After the base table is updated, the delayed IMV's after trigger is used to record the delta
> information in another table similar to the incremental log of the base table
> 2. When incremental refresh, use the data in the log instead of the data in the trasient table
> of the after trigger
> 3. We need to merge the incremental information in advance to ensure that the base_table
> after transaction filtering UNION ALL old_delta is the state before the base table is updated
> Case like below:
> Create table t( a int);
> ―begin to record log
> Insert into t select 1; ― newlog: 1 oldlog: empty
> Delete from t; ―newlog:1, oldlog：1
> ― begin to incremental refresh
> Select * from t where xmin < xid or (xmin = xid and cmin < cid); ― empty
> So this union all oldlog is not equal to before the base table is updated
> We need merge the incremental log in advance to make newlog: empty, oldlog: empty
>
> If implemented, incremental refresh must still be serialized, but the DML of the base table
> can not be blocked, that is to say, the base table can still record logs during incremental refresh,
> as long as we use same snapshot when incrementally updating.
>
> do you think there will be any problems with this solution?

I guess the deferred maintenance process would be basically what similar
to above. Especially, as you say, we need to merge incremental information
in some way before calculating deltas on the view. I investigated some
research papers, but I'll review again before working on deferred approach
design.

Regards,
Yugo Nagata

--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>