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

Readonly schema check path should not use ConsistencyLevel.ALL #37

Open
kulack opened this issue Feb 27, 2020 · 5 comments
Open

Readonly schema check path should not use ConsistencyLevel.ALL #37

kulack opened this issue Feb 27, 2020 · 5 comments
Assignees

Comments

@kulack
Copy link

kulack commented Feb 27, 2020

In the read only path (when the schema is already up to date), the cassandra queries used to determine that should not use ConsistencyLevel.ALL, they should use QUORUM or LOCAL_QUORUM.

Use case:
We automatically do a schema migration during app startup and always use backward compatible changes so under normal conditions, the schema migration code works great when any one of our app instances is deployed/started.

However, when our second data center is offline for maintenance (a semi normal situation) or nodes are down for problem determination or maintenance or whatever, it's a fairly normal case to bounce our applications. For example, it could be application problem contributing to what's going on.

So the read only path where CassandraVersioner.getCurrentVersion() responds with an version corresponding to 'everything is up to date' for MigrationEngine.Migrator.migrate() fails. We will be putting in some manual code to work around this but we'd like to not do that.
Thanks!

@jackmahoney
Copy link

Can confirm this is still an issue. For context when running migrations against amazon keyspaces (managed cassandra) only LOCAL_QUORUM is supported. Therefore the io.smartcast.migration.MigrationEngine migrate() function fails by calling CassandraVersioner

Caused by: com.datastax.driver.core.exceptions.InvalidQueryException: Consistency level ALL is not supported for this operation. Supported consistency levels are: ONE, LOCAL_QUORUM, LOCAL_ONE
        at com.datastax.driver.core.exceptions.InvalidQueryException.copy(InvalidQueryException.java:49) ~[cassandra-driver-core-3.7.2.jar!/:na]
        at com.datastax.driver.core.DriverThrowables.propagateCause(DriverThrowables.java:35) ~[cassandra-driver-core-3.7.2.jar!/:na]
        at com.datastax.driver.core.DefaultResultSetFuture.getUninterruptibly(DefaultResultSetFuture.java:293) ~[cassandra-driver-core-3.7.2.jar!/:na]
        at com.datastax.driver.core.AbstractSession.execute(AbstractSession.java:58) ~[cassandra-driver-core-3.7.2.jar!/:na]
        at io.smartcat.migration.CassandraVersioner.getCurrentVersion(CassandraVersioner.java:63) ~[cassandra-migration-tool-3.1.0.0.jar!/:na]
        at io.smartcat.migration.MigrationEngine$Migrator.migrate(M

@jackmahoney
Copy link

This is the offending method:

    public int getCurrentVersion(final MigrationType type) {
        final Statement select = QueryBuilder.select().all().from(SCHEMA_VERSION_CF)
                .where(QueryBuilder.eq(TYPE, type.name())).limit(1).setConsistencyLevel(ConsistencyLevel.ALL);
        final ResultSet result = session.execute(select);

        final Row row = result.one();
        return row == null ? 0 : row.getInt(VERSION);
    }

@mgobec
Copy link
Member

mgobec commented Aug 24, 2020

Hi @jackmahoney,

The reasoning behind ConsistencyLevel.ALL is that the cluster has to be up and in good state to run any schema changes. If that is not the case you can and will end up with schema inconsistency. This was done on purpose.
Regarding amazon managed Cassandra, are they advising to use the official (datastax) driver or is there a specific one (like for Astra)?

@mgobec mgobec self-assigned this Aug 24, 2020
@jackmahoney
Copy link

Hi @mgobec , you are probably right about the reason behind using ALL. It seems that some providers however disable it (like AWS Keyspaces). They recommend using the datastax driver as well. I ended up copy pasting the smartcat migration engine class and making the constructor accept a cassandraversioner - one I extending to override the getCurrentVersion. This appears to have worked and my migrations are now running against AWS Keyspaces. Perhaps a pragmatic solutions might be just to make the MigrationManager withSession() method also accept a custom versioner if provided. Thanks anyway have found my own solution for now.

@mgobec
Copy link
Member

mgobec commented Mar 17, 2022

Closed by #39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants