You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Benjamin Lerer (Jira)" <ji...@apache.org> on 2021/07/13 09:42:00 UTC
[jira] [Comment Edited] (CASSANDRA-15210) Streaming with CDC does
not honor cdc_enabled
[ https://issues.apache.org/jira/browse/CASSANDRA-15210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17379783#comment-17379783 ]
Benjamin Lerer edited comment on CASSANDRA-15210 at 7/13/21, 9:41 AM:
----------------------------------------------------------------------
Thanks [~azotcsit]
{quote}Despite the problem exists, it does not affect anything rather than performance (with unnecessary writing to commit log for CDC table on non-CDC nodes). I feel it is not so critical to make it mandatory for 4.0 release. WDYT?{quote}
It is not mandatory for 4.0.0 as it also affect 3.11 but it should be added to a 4.0.X patch release.
{quote}Even though the patch looks good, there are no unit tests for the change. Moreover, currently the affected class is not covered by tests. I trier to find a simple reference test, but looks like streaming tests are quite complicated. Do we need to have a test developed to get these changes merged? Even though it is 1 line patch, my belief that having a test is mandatory. Could you please share a reference test that can be used to cover CDC functionality in CassandraStreamReceiver?{quote}
For streaming we have several distributed tests. What I would suggest for this patch is an in-jvm DTest (see org.apache.cassandra.distributed.testStreamingTest for an example) that check both scenario with CDC enabled and disabled. It should be possible to check the effect of the configuration parameter by looking at the cdc directory.
[~aprudhomme] Are you interested in finishing your patch or should we reasign it ?
was (Author: blerer):
{quote}Despite the problem exists, it does not affect anything rather than performance (with unnecessary writing to commit log for CDC table on non-CDC nodes). I feel it is not so critical to make it mandatory for 4.0 release. WDYT?{quote}
It is not mandatory for 4.0.0 as it also affect 3.11 but it should be added to a 4.0.X patch release.
{quote}Even though the patch looks good, there are no unit tests for the change. Moreover, currently the affected class is not covered by tests. I trier to find a simple reference test, but looks like streaming tests are quite complicated. Do we need to have a test developed to get these changes merged? Even though it is 1 line patch, my belief that having a test is mandatory. Could you please share a reference test that can be used to cover CDC functionality in CassandraStreamReceiver?{quote}
For streaming we have several distributed tests. What I would suggest for this patch is an in-jvm DTest (see org.apache.cassandra.distributed.testStreamingTest for an example) that check both scenario with CDC enabled and disabled. It should be possible to check the effect of the configuration parameter by looking at the cdc directory.
[~aprudhomme] Are you interested in finishing your patch or should we reasign it ?
> Streaming with CDC does not honor cdc_enabled
> ---------------------------------------------
>
> Key: CASSANDRA-15210
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15210
> Project: Cassandra
> Issue Type: Bug
> Components: Consistency/Streaming, Feature/Change Data Capture
> Reporter: Andrew Prudhomme
> Assignee: Andrew Prudhomme
> Priority: Normal
>
> When SSTables are streamed for a CDC enabled table, the updates are processed through the write path to ensure they are made available through the commit log. However, currently only the CDC state of the table is checked. Since CDC is enabled at both the node and table level, a node with CDC disabled (with cdc_enabled: false) will unnecessarily send updates through the write path if CDC is enabled on the table. This seems like an oversight.
> I'd imagine the fix would be something like
>
> {code:java}
> - hasCDC = cfs.metadata.params.cdc;
> + hasCDC = cfs.metadata.params.cdc && DatabaseDescriptor.isCDCEnabled();{code}
> in
> org.apache.cassandra.db.streaming.CassandraStreamReceiver (4)
> org.apache.cassandra.streaming.StreamReceiveTask (3.11)
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org