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

Do not apply no-op schema updates #612

Merged
merged 2 commits into from
Feb 9, 2025

Conversation

rhuffy
Copy link
Contributor

@rhuffy rhuffy commented Feb 4, 2025

If a client submits a schema update via system_update_column_family, changes that are actually no-ops will still be announced to the cluster.

This PR skips this expensive schema update process if there is no difference in CFMetaData.

@rhuffy rhuffy marked this pull request as ready for review February 4, 2025 06:06
@wi11dey
Copy link
Contributor

wi11dey commented Feb 4, 2025

is there a risk here of skipping the update and the cluster permanently not achieving schema agreement on different uuids ? or has the uuid already been updated by this point

@andybradshaw
Copy link
Contributor

is there a risk here of skipping the update and the cluster permanently not achieving schema agreement on different uuids ? or has the uuid already been updated by this point

I don't think this would affect schema agreement, as the schema doesn't actually change. If it did, this wouldn't be safe to skip. The intent of this is to prevent us from even sending the gossip messages of the no-op, as the receiving node would not actually make any writes to the CF metadata, since the mutation in this message has no modifications. I was also concerned about this, but after reading through how these messages get applied, I think this should be safe.

AFAICT, it might be safe to never announce any mutations with empty modifications, although I'm not sure how often that happens (or how expensive the commitlog activity is when receiving them). But that's more risk than we need to take to resolve the immediate issue.

@wi11dey
Copy link
Contributor

wi11dey commented Feb 5, 2025

ah ack. i think would be good to test on an rc and try upgrading/downgrading the old atlas to double check

@rhuffy rhuffy merged commit e2f471a into palantir-cassandra-2.2.18 Feb 9, 2025
7 checks passed
@rhuffy rhuffy deleted the rh/ignore-noop-schema-change branch February 9, 2025 06:24
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.

3 participants