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

Add support for customized Cassandra keyspace creations #15821

Open
rreimann opened this issue Jan 31, 2019 · 21 comments
Open

Add support for customized Cassandra keyspace creations #15821

rreimann opened this issue Jan 31, 2019 · 21 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@rreimann
Copy link

Using the Cluster bean from CassandraAutoConfiguration makes it quite difficult to provide a custom keyspace creation (e.g. via CreateKeyspaceSpecification).

While CassandraClusterFactoryBean#setKeyspaceCreations provides support for custom keyspace creations I couldn't find anything similar when the cluster is created via the auto-configuration.

Trying to find a workaround that doesn't duplicate existing auto-configuration code led to this ugly custom configuration...

@Configuration
@Import(CassandraDataAutoConfiguration.class)
@EnableConfigurationProperties(CassandraProperties.class)
public class CassandraCustomConfiguration {

    @Bean
    Cluster cluster(CassandraProperties properties, ObjectProvider<ClusterBuilderCustomizer> builderCustomizers) {
        CassandraAutoConfiguration cassandraAutoConfiguration = new CassandraAutoConfiguration(properties, builderCustomizers);
        Cluster cluster = cassandraAutoConfiguration.cassandraCluster();
        createKeyspace(cluster, getSimpleKeyspaceCreation());
        return cluster;
    }

    private CreateKeyspaceSpecification getSimpleKeyspaceCreation() {
        return CreateKeyspaceSpecification.createKeyspace("foo").ifNotExists(); // simplified sample
    }

    private void createKeyspace(Cluster cluster, CreateKeyspaceSpecification keyspaceCreation) {
        try (Session session = cluster.connect()) {
            CqlTemplate template = new CqlTemplate(session);
            template.execute(CreateKeyspaceCqlGenerator.toCql(keyspaceCreation));
        }
    }
}

It would be nice if the auto-configuration would provide support for keyspace creations.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 31, 2019
@snicoll
Copy link
Member

snicoll commented Jan 31, 2019

Thanks for the suggestion. I don't know if that's the code you are using now but importing the auto-configurations the way you do is quite unusual (and probably unnecessary) . What is wrong with getting the Cluster as a bean and creating the keyspace?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Jan 31, 2019
@rreimann
Copy link
Author

rreimann commented Jan 31, 2019

Thanks for the quick response...

but importing the auto-configurations the way you do is quite unusual (and probably unnecessary)

yes I know that's why i called it "ugly" myself. The import was necessary because CassandraDataAutoConfiguration is disabled due to the Cluster bean I provide in my config.

What is wrong with getting the Cluster as a bean and creating the keyspace?

Trying that failed for me because CassandraDataAutoConfiguration#cassandraSession invokes getCluster().connect(keyspaceName) via CassandraCqlSessionFactoryBean#afterPropertiesSet which fails due to the non-existing keyspace before I have a chance to get the Cluster injected and create the keyspace.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 31, 2019
@snicoll
Copy link
Member

snicoll commented Feb 1, 2019

The import was necessary because CassandraDataAutoConfiguration is disabled due to the Cluster bean I provide in my config.

I don't see why it was. If you provide your own Cluster you are basically providing your opinion and Spring Boot will back off which is the advertized feature. There is a @ConditionalOnMissingBean on the cluster @Bean method so providing your own will only disable that part of the auto-config.

which fails due to the non-existing keyspace before I have a chance to get the Cluster injected and create the keyspace.

Thanks that helps.

@rreimann
Copy link
Author

rreimann commented Feb 1, 2019

I don't see why it was. If you provide your own Cluster you are basically providing your opinion and Spring Boot will back off which is the advertized feature. There is a @ConditionalOnMissingBean on the cluster @bean method so providing your own will only disable that part of the auto-config.

You're right, my bad - there's no need for the import in prod code. I checked again and it was only a test that run in to a NoSuchBeanDefinitionException: No qualifying bean of type 'org.springframework.data.cassandra.core.CassandraOperations due to a restricted test context.

@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Apr 17, 2019
@rreimann
Copy link
Author

@philwebb would you like to share the reason why this was declined?

@snicoll
Copy link
Member

snicoll commented Apr 19, 2019

@rreimann because we were discussing two different topics in one issue and I guess @philwebb concluded that you found out what the problem was. But this issue is about keyspace creation so let's leave it at that from now on.

@snicoll snicoll reopened this Apr 19, 2019
@snicoll snicoll added status: waiting-for-triage An issue we've not yet triaged and removed status: declined A suggestion or change that we don't feel we should currently apply labels Apr 19, 2019
@rreimann
Copy link
Author

👍 o.k. - thanks for clarifying and reopening.

@iNikem

This comment has been minimized.

@snicoll

This comment has been minimized.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Sep 5, 2019
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Sep 6, 2019
@wilkinsona wilkinsona added this to the 2.x milestone Sep 6, 2019
@wilkinsona
Copy link
Member

We'll look at offering first-class support for this in a future 2.x release. In the meantime, I think it can be achieved reasonably cleanly using a BeanPostProcessor similar to the following:

@Bean
public static BeanPostProcessor keyspaceCreator() {
    return new BeanPostProcessor() {

        @Override
        public Object postProcessAfterInitialization(Object bean, String beanName) {
            if (bean instanceof Cluster) {
                Cluster cluster = (Cluster) bean;
                try (Session session = cluster.connect()) {
                    session.execute(CreateKeyspaceCqlGenerator.toCql(getSimpleKeyspaceCreation()));
                }
            }
            return bean;
        }

        private CreateKeyspaceSpecification getSimpleKeyspaceCreation() {
            return CreateKeyspaceSpecification.createKeyspace("foo").ifNotExists();
        }

    };
}

@rreimann If you've already tried something like this and it didn't work, I'd be very interested to hear about the problems that you had.

@eddumelendez
Copy link
Contributor

Hi, I would like to take this issue with a friend of mine (@manduinca) but I have been checking that there are many *Specification classes. The issue's title is specific to the keyspace but I would like to ask first if it is going to be kept just for keyspace creation because may someone else in the future could request something similar to this but for tables, users and son on which would sound to me like a schema data migration.

just pinging to @mp911de in case I am missing something

@mp911de
Copy link
Member

mp911de commented Oct 2, 2019

I'm not convinced of creating a keyspace on startup is a use-case worth supporting. It compares to CREATE DATABASE when using JDBC. Databases (keyspaces) should be present when connecting to these.

There is another point to consider which is the evolution of the Cassandra driver (version 4.x). In Cassandra driver 3.x and earlier, the driver makes a distinction between a Cluster and a Session object. Sessions are obtained from Cluster and that arrangement allows interacting with Cluster before creating the Session for application usage.

With Cassandra driver 4, the setup changes to:

CqlSession session = CqlSession.builder().build();
ResultSet rs = session.execute("select release_version from system.local");

There's no longer an intermediate Cluster object but the session object points directly to the right keyspace.

Please also note that CreateKeyspaceSpecification classes originate from Spring Data for Apache Cassandra that is an optional dependency.

I'd suggest revisiting schema migration in the context of the Cassandra driver 4 migration. Check out DATACASS-187 for further reference.

@eddumelendez
Copy link
Contributor

Thanks for the quick reply @mp911de !

Please also note that CreateKeyspaceSpecification classes originate from Spring Data for Apache Cassandra that is an optional dependency.

Good point

I'd suggest revisiting schema migration in the context of the Cassandra driver 4 migration. Check out DATACASS-187 for further reference.

Thanks for the reference

@bcalmac
Copy link

bcalmac commented Dec 9, 2021

@wilkinsona @snicoll With version 4 of the Cassandra driver the Cluster class goes away and instantiating the CqlSession fails as below when the keyspace is missing:

Failed to instantiate [com.datastax.oss.driver.api.core.CqlSession]: Factory method 'cassandraSession' threw exception; 
nested exception is com.datastax.oss.driver.api.core.InvalidKeyspaceException: Invalid keyspace clearcore

The best solution I could find was to override the cassandraSession bean and create the keyspace just before instantiating the bean. Note that I have to clear and then restore keyspaceName on the builder.

    @Bean
    public CqlSession cassandraSession(CqlSessionBuilder cqlSessionBuilder, CassandraProperties properties) {
        String keyspaceName = properties.getKeyspaceName();
        // keyspaceName must be null for the session creating the keyspace
        try (CqlSession session = cqlSessionBuilder.withKeyspace((CqlIdentifier) null).build()) {
            session.execute(CreateKeyspaceCqlGenerator.toCql(
                    CreateKeyspaceSpecification.createKeyspace(keyspaceName).ifNotExists()));
        }
        // restore keyspaceName before creating the session
        return cqlSessionBuilder.withKeyspace(keyspaceName).build();
    }

Do you see a more elegant option? Would this be a good time to provide this functionality out of the box? When running Cassandra for integration tests or local development you typically get a barebones container and it's convenient to (optionally) have the ability to create the keyspace at startup.

@bcalmac
Copy link

bcalmac commented Dec 9, 2021

Another reason for adding keyspace creation is feature parity with the configuration from spring-data-cassandra. This functionality is currently available through org.springframework.data.cassandra.config.AbstractSessionConfiguration#getKeyspaceCreations with the implementation in org.springframework.data.cassandra.config.CqlSessionFactoryBean#initializeCluster. I would assume that if one is to use the spring-boot auto-configuration for Cassandra then it's not a good practice to mix it with the configuration from spring-data-cassandra.

@wilkinsona
Copy link
Member

The Spring Data team advised against supporting this in the past. What's your take on this now please, @mp911de, now that Boot's using the 4.x driver?

@wilkinsona wilkinsona added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Dec 9, 2021
@bcalmac
Copy link

bcalmac commented Dec 10, 2021

If other people have the same need, here is my final solution in the form of an auto-configuration that I've added to our company's starter. I've also introduced a property to make the functionality opt-in.

/**
 * create the configured keyspace before the first cqlSession is instantiated. This is guaranteed by running this
 * autoconfiguration before the spring-boot one.
 */
@ConditionalOnClass(CqlSession.class)
@ConditionalOnProperty(name = "spring.data.cassandra.create-keyspace", havingValue = "true")
@AutoConfigureBefore(CassandraAutoConfiguration.class)
public class CassandraCreateKeyspaceAutoConfiguration {

    private static final Logger logger = LoggerFactory.getLogger(CassandraCreateKeyspaceAutoConfiguration.class);

    public CassandraCreateKeyspaceAutoConfiguration(CqlSessionBuilder cqlSessionBuilder, CassandraProperties properties) {
        // It's OK to mutate cqlSessionBuilder because it has prototype scope.
        try (CqlSession session = cqlSessionBuilder.withKeyspace((CqlIdentifier) null).build()) {
            logger.info("Creating keyspace {} ...", properties.getKeyspaceName());
            session.execute(CreateKeyspaceCqlGenerator.toCql(
                    CreateKeyspaceSpecification.createKeyspace(properties.getKeyspaceName()).ifNotExists()));
        }
    }
}

@mp911de
Copy link
Member

mp911de commented Dec 10, 2021

Keyspace creation is typically an operations task similar to SQL databases where the application connects to an existing database instead of creating one. Therefore, this feature is not a first-class citizen.

Spring Data Cassandra's CqlSessionFactoryBean is the successor of ClusterFactoryBean. With the migration to the new driver, we didn't want to take away keyspace creation entirely as it would break applications in a sense of requiring them to come up with a replacement for keyspace creation. We found that such a burden would be a too big ask hence you find keyspace creation leftovers in Spring Data Cassandra.

@wilkinsona wilkinsona removed the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Dec 10, 2021
@snanduri7
Copy link

Having the support in spring data for creating keyspace is encouraging. However, I don't see support for a small property "ifNotExists" while creating keyspace.

I am using spring data cassandra ver 3.3.1. Able to successfully create keyspace on first run. However on rerunning the app I get exception com.datastax.oss.driver.api.core.servererrors.AlreadyExistsException. I am aware that by overriding the behavior of Bean class I could get past this. But would like to know if a solution already exist or will it be provided in XML config.

Thank You.

@pluttrell
Copy link

I agree with @mp911de's comment, especially in the context of production systems or already running environments.

Keyspace creation is typically an operations task similar to SQL databases where the application connects to an existing database instead of creating one.

But Spring Boot has long provided great support for unit testing, which is where this feature is needed.

The core issue is that the DataStax v4 driver requires that the keyspace be created ahead of time or it fails.

Docker has become a core part of many companies workflows and with the ease at which we can automate, the expectations have changed as well, such that there is less tolerance for manual configuration - especially for CI environments.

Most Cassandra containers come without any keyspace created, and not all of the automation tooling provide an easy way to inject it into the workflow: e.g. (1) start a container, (2) create a keyspace, then (3) run tests.

I've tried @bcalmac's clever constructor-based auto-configuration, but haven't been able to get it to run before Spring Boot's CassandraAutoConfiguration even though it declares @AutoConfigureBefore(CassandraAutoConfiguration.class). I believe this is getting ignored because the CqlSessionBuilder dependency is created within CassandraAutoConfiguration, so it runs first and then also creates the CqlSession, which fails because the keyspace hasn't been created.

I think Spring Boot has an opportunity to make unit tests against Cassandra containers much easier by auto-creating the keyspace if it's not already there - perhaps based on a property as @bcalmac suggested.

@mhalbritter
Copy link
Contributor

mhalbritter commented Aug 11, 2022

I can't get the solution from @bcalmac working as well, and I solved it like this in my tests:

@Configuration
class CassandraConfiguration {

	@Bean
	CqlSession cqlSession(CqlSessionBuilder cqlSessionBuilder, CassandraProperties properties) {
		// This creates the keyspace on startup
		try (CqlSession session = cqlSessionBuilder.build()) {
			session.execute(CreateKeyspaceCqlGenerator
					.toCql(CreateKeyspaceSpecification.createKeyspace(properties.getKeyspaceName()).ifNotExists()));
		}
		return cqlSessionBuilder.build();
	}

}

They too use docker, and I agree with @pluttrell that triggering a key space creation via properties would have spared me the headache of finding out how to do this programmatically. This feature might not be needed in production arrangements, but would be a boon to integration testing.

@philwebb philwebb modified the milestones: 2.x, 3.x Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests