[
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-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
