Skip to content

CASSSIDECAR-451: Sidecar CDC should resolve real keyspace replication factors from the driver#343

Open
Klose6 wants to merge 3 commits into
apache:trunkfrom
Klose6:CASSSIDECAR-451
Open

CASSSIDECAR-451: Sidecar CDC should resolve real keyspace replication factors from the driver#343
Klose6 wants to merge 3 commits into
apache:trunkfrom
Klose6:CASSSIDECAR-451

Conversation

@Klose6
Copy link
Copy Markdown

@Klose6 Klose6 commented May 6, 2026

Currently the analytics library uses ReplicationFactorSupplier.DEFAULT (SimpleStrategy/RF=1), which causes peer discovery to find only one replica per token range, it is better to add the support to resolve real keyspace replication factors from the driver.

Copy link
Copy Markdown
Contributor

@jberragan jberragan left a comment

Choose a reason for hiding this comment

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

Thanks for adding, not sure why this was neglected in the Sidecar implementation.

@Klose6
Copy link
Copy Markdown
Author

Klose6 commented May 13, 2026

Thanks for adding, not sure why this was neglected in the Sidecar implementation.

yeah, we found this during the cdc e2e testing.

@Klose6
Copy link
Copy Markdown
Author

Klose6 commented May 14, 2026

@jberragan @bbotella could you take a look when you got a chance? thanks in advance!

@bbotella
Copy link
Copy Markdown
Contributor

Hi!! Really sorry for the delay!! I've been out traveling for a while, and just made it back. I'll be taking a look this week.

@Klose6
Copy link
Copy Markdown
Author

Klose6 commented May 18, 2026

Hi!! Really sorry for the delay!! I've been out traveling for a while, and just made it back. I'll be taking a look this week.

np, thank you so much!

@Klose6 Klose6 force-pushed the CASSSIDECAR-451 branch from 84d35cf to bdd8bdf Compare May 19, 2026 17:12
@jyothsnakonisa
Copy link
Copy Markdown
Contributor

jyothsnakonisa commented May 19, 2026

@Klose6 Thanks for the patch, I have this change as part of another PR #347 Which is bringing in several changes for getting CDC working. Can we please wait for it to be merged? If there is anything else that you need to add in addition to that, we can work on getting those changes in.

@Klose6
Copy link
Copy Markdown
Author

Klose6 commented May 20, 2026

@Klose6 Thanks for the patch, I have this change as part of another PR #347 Which is bringing in several changes for getting CDC working. Can we please wait for it to be merged? If there is anything else that you need to add in addition to that, we can work on getting those changes in.

@jyothsnakonisa Sure I’ll wait for your PR to be merged first since it already contains my change. After that, I can rebase/update my branch accordingly if needed(but overall looks my changes is not needed after your PR). Thanks for coordinating this.
cc: @bbotella @jberragan

@jyothsnakonisa
Copy link
Copy Markdown
Contributor

@Klose6 I merged my PR to fix CDC issues yesterday, can u rebase and see if you need to add anything?

@Klose6
Copy link
Copy Markdown
Author

Klose6 commented May 21, 2026

@Klose6 I merged my PR to fix CDC issues yesterday, can u rebase and see if you need to add anything?

I just did a rebase and overall looks good, no need to add extra things, your PR uses the SchemaSupplier for the CDC enabled tables and my PR uses the InstanceMetadataFetcher, both will work but SchemaSupplier is simpler, I think we can just close this PR, thank you!

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.

5 participants