[ 
https://issues.apache.org/jira/browse/CASSANDRA-13762?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16323891#comment-16323891
 ] 

Andrés de la Peña commented on CASSANDRA-13762:
-----------------------------------------------

Overall the patch looks good to me. A few insignificant nits:

* The new {{StorageService.finishViewBootstrap}} method [triggers a view 
build|https://github.com/pauloricardomg/cassandra/blob/c47cfb9f3299274f97e47610eb6329390dcacdc0/src/java/org/apache/cassandra/service/StorageService.java#L1550]
 if the batchlog replay fails. It could be good to simulate this case in a 
dtest, WDYT?
* The [second {{return}} 
statement|https://github.com/pauloricardomg/cassandra/blob/c47cfb9f3299274f97e47610eb6329390dcacdc0/src/java/org/apache/cassandra/repair/SystemDistributedKeyspace.java#L314]
 in {{SystemDistributedKeyspace.isViewBuilt}} seems unnecessary.
* The new unit test {{testhasBuiltAllViews}} should be named 
{{testHasBuiltAllViews}}, and the create view statement could be splitted for 
better readability.
* The [last {{if}} block 
statement|https://github.com/pauloricardomg/cassandra-dtest/blob/fa280103530461447ec9b92e3ecf9f023a42f0fe/materialized_views_test.py#L1240-L1245]
 of the new dtest method [doesn't use the log 
marking|https://github.com/pauloricardomg/cassandra-dtest/blob/fa280103530461447ec9b92e3ecf9f023a42f0fe/materialized_views_test.py#L1243].
 
* I think it could be useful to extend [this dtest debug 
message|https://github.com/pauloricardomg/cassandra-dtest/blob/fa280103530461447ec9b92e3ecf9f023a42f0fe/materialized_views_test.py#L1241]
 with the reason of the restart, something like {{"Restart the cluster to 
finish the stopped build"}}.

> Ensure views created during (or just before) range movements are properly 
> built
> -------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-13762
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-13762
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Materialized Views
>            Reporter: Paulo Motta
>            Assignee: Paulo Motta
>            Priority: Minor
>              Labels: materializedviews
>         Attachments: trunk-13762-dtest.png, trunk-13762-testall.png
>
>
> CASSANDRA-13065 assumes the source node has its views built to skip running 
> base mutations through the write path during range movements.
> It is possible that the source node has not finished building the view, or 
> that a new view is created during a range movement, in which case the view 
> may be wrongly marked as built on the destination node.
> The former problem was introduced by #13065, but even before that a view 
> created during a range movement may not be correctly built on the destination 
> node because the view builder will be triggered before it has finished 
> streaming the source data, wrongly marking the view as built on that node.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to