[
https://issues.apache.org/jira/browse/CASSANDRA-11500?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16147044#comment-16147044
]
Paulo Motta commented on CASSANDRA-11500:
-----------------------------------------
Thanks for the update! See follow-up below:
bq. My understanding is: view row is strict iff the view has non-key base row
as view pk. When it's Strict, the view's row liveness/deletion should use this
non-key base column's timestamp as well as ttl, unless there is a greater row
deletion.(It's like a simplified version of "VirtualCells" which only store
metadata for non-key base column in view pk)
That's a good simplification of virtual cells which should allow us to fix the
out-of-order update issues with non-base PK column
(CASSANDRA-11500,CASSANDRA-13657).
I wasn't very comfortable with our previous approach of enforcing strict
liveness during row merge, since it changes a lot of low-level
structures/interfaces (like BTreeRow/MergeListener, etc) to enforce a
table-level setting. Since we'll probably get rid of this when doing a proper
implementation of virtual cells , I updated [on this
commit|https://github.com/pauloricardomg/cassandra/commit/2b7b7c8ca79b6ef72c7c92535096c6d1d899ee43]
to perform the filtering during read instead which will give us the same
result but with less change in unrelated code. Do you see any problem with this
approach?
bq. this wouldn't support complex cases as explained above. eg. c/d unselected,
update c@10, delete c@11, update d@5. view row should be alive but dead
Actually this is a bit worst than I initially thought, since unselected
deletion can also shadow a previous insert (not only update) so I
[added|https://github.com/pauloricardomg/cassandra/commit/4200db215bdd6a9338e5d16cc567444537104e4b]
an additional test case to testPartialDeleteUnselectedColumn with this
scenario. I also
[added|https://github.com/pauloricardomg/cassandra/commit/623d5f6d935b57ad3c949206b85206aea15a8844]
a note to both NEWS.txt and to the documentation explaining that it might be
unsafe to perform deletion on unselected view column. I created CASSANDRA-13826
to add proper support to this and other cases with storage engine changes.
bq. If there is no non-pk base column in view-pk, the view's liveness/deletion
is using max of base livenessIno + unselected column. unselected column's ttl
is used only when it affects view row liveness. Selected columns won't
contribute to livenessInfo or row deletion.
In order to prevent drop of unselected column to keep view row alive, I added a
new
[commit|https://github.com/pauloricardomg/cassandra/commit/afef09233a9ee0657104319de4c8a4f76e9ad292]
disallowing drop of base table columns until we can deal with this properly.
bq. Current shadowable tombstone is not used to avoid the issue of
resurrecting deleted cells. We will expired-livenessInfo instead.
One problem of replacing shadowable tombstones by expired liveness info is that
it stores an additional unused ttl field for every shadowed view entry to solve
the commutative view deletion problem. In order to avoid this I [updated the
patch|https://github.com/pauloricardomg/cassandra/commit/e0da138ab10f6c0fc014de86fb251e11358d80cc]
to only use expired ttl when a shadowable tombstone would not work along with
an explanation on why that is used since it's a hack.
bq. in TableViews.java, the DeletionTracker should be applied even if existing
has no data, eg. partition-deletion
Is this still required after enforcing strict liveness on the
[PurgeFunction|https://github.com/pauloricardomg/cassandra/commit/2b7b7c8ca79b6ef72c7c92535096c6d1d899ee43#diff-5636ce30e505443b3e24a1a6ba55e476R112]?
I removed this and also
[simplified|https://github.com/apache/cassandra/commit/3188feb38fb63c5ca556a8273aadc84571ac1bb6]
{{ViewUpdateGenerator.deleteOldEntry}} to always use shadowable deletion and
it didn't seem to affect any test. Do you see any other case which could be
affected by this?
Also, as we discussed offline, since we are changing the row liveness/deletion
computation when the view has non-key base row as view PK, this means that an
older update to this column may not generate a correct shadowable deletion in
the view, as shown by the example below:
{noformat}
before upgrade
base: (k=1@5) a=1@1 b=1@1 c=1@1
view: (k=1 && a=1 @5) b=1@1 c=1@1 (old way of computing livenessInfo
timestamp, max of view pk columns)
___________
after upgrade
update a = null @ 1
base: (k=1@5) a=null@1 b=1@1 c=1@1
view:
(k=1 && a=1 @5) b=1@1 c=1@1 (old way of computing livenessInfo
timestamp, max of view pk columns)
(k=1 && a=1 @expired at 1) (new way of computing
livenessInfo timestamp, non-key base column in view pk dominates)
new expired livenessInfo cannot shadow old view row if the removal
time is less the previous max
{noformat}
Even though this should be a pretty unlikely scenario, it would only happen if
a missed update that shadows a view PK column arrive after upgrade, which can
typically happen after a hint or repair. In order to prevent against this, I
added a
[note|https://github.com/pauloricardomg/cassandra/commit/623d5f6d935b57ad3c949206b85206aea15a8844#diff-4302f2407249672d7845cd58027ff6e9R75]
to the {{Upgrading}} section of {{NEWS.txt}} explaining about this caveat and
that running repair before the upgrade should be sufficient to avoid it.
[~jasonstack] I rebased your patch on current trunk and added the above
suggestions on top of it, can you take a look and let me know what do you
think? Patch and tests available below:
||trunk||dtest||
|[branch|https://github.com/apache/cassandra/compare/trunk...pauloricardomg:trunk-11500]|[branch|https://github.com/riptano/cassandra-dtest/compare/master...pauloricardomg:11500]|
|[testall|http://jenkins-cassandra.datastax.lan/view/Dev/view/paulomotta/job/pauloricardomg-trunk-11500-testall/lastCompletedBuild/testReport/]|
|[dtest|http://jenkins-cassandra.datastax.lan/view/Dev/view/paulomotta/job/pauloricardomg-trunk-11500-dtest/lastCompletedBuild/testReport/]|
> Obsolete MV entry may not be properly deleted
> ---------------------------------------------
>
> Key: CASSANDRA-11500
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11500
> Project: Cassandra
> Issue Type: Bug
> Components: Materialized Views
> Reporter: Sylvain Lebresne
> Assignee: ZhaoYang
> Fix For: 3.0.x, 3.11.x, 4.x
>
>
> When a Materialized View uses a non-PK base table column in its PK, if an
> update changes that column value, we add the new view entry and remove the
> old one. When doing that removal, the current code uses the same timestamp
> than for the liveness info of the new entry, which is the max timestamp for
> any columns participating to the view PK. This is not correct for the
> deletion as the old view entry could have other columns with higher timestamp
> which won't be deleted as can easily shown by the failing of the following
> test:
> {noformat}
> CREATE TABLE t (k int PRIMARY KEY, a int, b int);
> CREATE MATERIALIZED VIEW mv AS SELECT * FROM t WHERE k IS NOT NULL AND a IS
> NOT NULL PRIMARY KEY (k, a);
> INSERT INTO t(k, a, b) VALUES (1, 1, 1) USING TIMESTAMP 0;
> UPDATE t USING TIMESTAMP 4 SET b = 2 WHERE k = 1;
> UPDATE t USING TIMESTAMP 2 SET a = 2 WHERE k = 1;
> SELECT * FROM mv WHERE k = 1; // This currently return 2 entries, the old
> (invalid) and the new one
> {noformat}
> So the correct timestamp to use for the deletion is the biggest timestamp in
> the old view entry (which we know since we read the pre-existing base row),
> and that is what CASSANDRA-11475 does (the test above thus doesn't fail on
> that branch).
> Unfortunately, even then we can still have problems if further updates
> requires us to overide the old entry. Consider the following case:
> {noformat}
> CREATE TABLE t (k int PRIMARY KEY, a int, b int);
> CREATE MATERIALIZED VIEW mv AS SELECT * FROM t WHERE k IS NOT NULL AND a IS
> NOT NULL PRIMARY KEY (k, a);
> INSERT INTO t(k, a, b) VALUES (1, 1, 1) USING TIMESTAMP 0;
> UPDATE t USING TIMESTAMP 10 SET b = 2 WHERE k = 1;
> UPDATE t USING TIMESTAMP 2 SET a = 2 WHERE k = 1; // This will delete the
> entry for a=1 with timestamp 10
> UPDATE t USING TIMESTAMP 3 SET a = 1 WHERE k = 1; // This needs to re-insert
> an entry for a=1 but shouldn't be deleted by the prior deletion
> UPDATE t USING TIMESTAMP 4 SET a = 2 WHERE k = 1; // ... and we can play this
> game more than once
> UPDATE t USING TIMESTAMP 5 SET a = 1 WHERE k = 1;
> ...
> {noformat}
> In a way, this is saying that the "shadowable" deletion mechanism is not
> general enough: we need to be able to re-insert an entry when a prior one had
> been deleted before, but we can't rely on timestamps being strictly bigger on
> the re-insert. In that sense, this can be though as a similar problem than
> CASSANDRA-10965, though the solution there of a single flag is not enough
> since we can have to replace more than once.
> I think the proper solution would be to ship enough information to always be
> able to decide when a view deletion is shadowed. Which means that both
> liveness info (for updates) and shadowable deletion would need to ship the
> timestamp of any base table column that is part the view PK (so {{a}} in the
> example below). It's doable (and not that hard really), but it does require
> a change to the sstable and intra-node protocol, which makes this a bit
> painful right now.
> But I'll also note that as CASSANDRA-1096 shows, the timestamp is not even
> enough since on equal timestamp the value can be the deciding factor. So in
> theory we'd have to ship the value of those columns (in the case of a
> deletion at least since we have it in the view PK for updates). That said, on
> that last problem, my preference would be that we start prioritizing
> CASSANDRA-6123 seriously so we don't have to care about conflicting timestamp
> anymore, which would make this problem go away.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
