Skip to content

CASSSIDECAR-492: CDC log_only mode should not require Kafka producer … - #389

Merged
jyothsnakonisa merged 2 commits into
apache:trunkfrom
shruti-p-s:CASSSIDECAR-492
Oct 10, 2026
Merged

jyothsnakonisa merged 2 commits into
apache:trunkfrom
shruti-p-s:CASSSIDECAR-492

Conversation

@shruti-p-s

Copy link
Copy Markdown
Contributor

…configuration
Patch by Shruti Sekaran; Reviewed by TBD for CASSSIDECAR-492

cdcConfigs();
return cdcConfigAccessor.isAvailable()
&& !kafkaConfigMappings.isEmpty()
&& (logOnly() || !kafkaConfigMappings.isEmpty())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This starts to feel a little bit convoluted (not this PR's fault).

CEP-44 offers Cdc over kafka, but, kafka is just a data sink we are using for CDC. Logs could be just another one. What do you think about a broader refactor to support kafka, logs, or any other type of "sending CDC events"?, that could even work in parallel?

For example:

  • Log AND kafka
  • Log AND AWS SNS

Having said that, I understand the scope of this proposal may be a bit bigger, and I guess it'd be ok to have it as part of another Jira, but, what do folks think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave the refactoring in a dedicated JIRA.

Sharing my thoughts — see if they make sense.

Log is not a data sink. It is a by-product of the process. The patch is to introduce a mode to avoid publishing to data sink, which is kafka specifically as of today.

If we want to introduce other data sink options, (yet to be discussed), the selection would be among the data sinks. Log is configured separately.


if (conf.logOnly())
{
LOGGER.warn("CDC log_only is set; mutations will be logged and not published to Kafka");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be an info? Configuration has been explicitly selected for this.

@jyothsnakonisa jyothsnakonisa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, can u rebase the PR and also run CI using circle CI and paste the link

@bbotella

bbotella commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

+1

@shruti-p-s

Copy link
Copy Markdown
Contributor Author

@jyothsnakonisa
jyothsnakonisa merged commit f8fee7b into apache:trunk Oct 10, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants