Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes issue-257: Sets write consistency on Batch statement #258

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arvy
Copy link

@arvy arvy commented Apr 10, 2024

Sets consistency level on batch statements:

Which issue(s) this PR fixes:
Fixes #257

I have tested this manually, using query tracing - I have validated that this is indeed an issue and this PR does fix it.
Any ideas for an automated test welcome :)

Steps for manual acceptance test:

  1. purge any existing trace data (cqlsh> TRUNCATE system_traces.sessions; TRUNCATE system_traces.events;)
  2. enable query tracing on TARGET cluster (nodetool settraceprobability 1.0)
  3. migrate a table
  4. take note of CL on the batch query in the traced queries (cqlsh> select * from system_traces.sessions)
 61d387d0-f6f4-11ee-8c57-319793ec9a4a | 10.166.64.186 |   QUERY | 10.166.68.115 |     1161 |                                                                       {'consistency_level': 'LOCAL_QUORUM', 'serial_consistency_level': 'SERIAL'} | Execute batch of CQL3 queries | 2024-04-10 04:39:56.877000+0000

@arvy arvy requested a review from a team as a code owner April 10, 2024 05:26
@msmygit
Copy link
Collaborator

msmygit commented Apr 10, 2024

@arvy could you update the manual testing steps on this PR description for future reference? Thanks for the PR!

@msmygit msmygit enabled auto-merge (squash) April 10, 2024 10:49
Copy link
Collaborator

@pravinbhat pravinbhat left a comment

Choose a reason for hiding this comment

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

Hi, Thanks for looking into this, however I think it is not needed as the individual statements within the batch are already set with the right consistency-level. See TargetUpdateStatement & TargetInsertStatement for details.

Did you observe an issue with the write-consistency?

@pravinbhat
Copy link
Collaborator

pravinbhat commented Apr 10, 2024

Hi, Thanks for looking into this, however I think it is not needed as the individual statements within the batch are already set with the right consistency-level. See TargetUpdateStatement & TargetInsertStatement for details.

Did you observe an issue with the write-consistency?

Sorry, I did not see your earlier comments, & looks like batches are ignoring consistency on individual statements.
I would recommend we remove the getWriteConsistencyLevel() call if you are not using it. Thanks!

@arvy
Copy link
Author

arvy commented Apr 10, 2024

More context on my urgency: Sony has described many issues with inconsistent discrepancies when running validation job, which were most likely due to this bug (Datastax support case 00091967)

@pravinbhat
Copy link
Collaborator

More context on my urgency: Sony has described many issues with inconsistent discrepancies when running validation job, which were most likely due to this bug (Datastax support case 00091967)

I would be suspicious if you are seeing many issues as we have not found evidence of such issues for other customers & we have done many migrations as well as validations for very large clusters. If the target env is Astra, then write consistency issue is not possible as Astra will auto use local-quorum for any write operations (as lower consistencies are not applicable to Astra). Can you elaborate the env details (origin, target, etc.) where you are seeing consistency issues.

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.

Consistency levels are always LOCAL_ONE
5 participants