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

Clickhouse fixes for cluster operation #17

Merged

Conversation

Chris3LC
Copy link
Contributor

@Chris3LC Chris3LC commented Feb 5, 2024

I tried to make the clickhouse community plugin work on our setup, but had to make a few changes:

  • Adding "ON CLUSTER" to DDL's, to have the changes reproduced on all nodes.
  • Adding a systemConnection in ClickHouseDatabase - this one is a bit surprising to me, but I got a "Database the_database doesn't exist" error when executing jdbcTemplate.queryForInt("SELECT COUNT() FROM system.tables WHERE database = ?", name). Even though I am not accessing the_database, the connection is configured to use it, so I get an exception.
  • Using getCatalog instead of getSchema. This is kinda optional, but it appears that with clickhouse-jdbc:0.5.0 and newer versions, catalog is used instead of schema (can be overridden using ?databaseTerm=schema in the jdbc-url).

Each change is in a separate commit.

@Chris3LC
Copy link
Contributor Author

Chris3LC commented Feb 5, 2024

@sazonov Could you please review this change?

This is approximately the setup we used to run it (kotlin):

    Flyway(
        Flyway.configure()
            .dataSource(
                "jdbc:clickhouse://localhost:9001/", // change port to 9002, 9003, 9004 for the other nodes of the cluster
                "admin",
                "123"
            )
            .configuration(mapOf(
                "flyway.clickhouse.clusterName" to "'{cluster}'",
                // don't put {shard} in the zookeeperPath, to have the change replicated (and visible) on all nodes
                "flyway.clickhouse.zookeeperPath" to "/clickhouse/tables/{database}/{table}",
            ))
            .placeholders(mapOf("database" to "the_database"))
            .schemas("the_database")
            .createSchemas(true)
            .failOnMissingLocations(true)
            .locations("clickhouse/migrations")
            .table("flyway_history")
            .baselineVersion("0000")
            .baselineOnMigrate(true)
    ).migrate()

I tested using following docker-compose cluster: https://github.com/Chris3LC/clickhouse-cluster

Copy link
Contributor

@sazonov sazonov left a comment

Choose a reason for hiding this comment

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

I tested this on single node and cluster instances. It works well, thanks for the fixes.

@sazonov
Copy link
Contributor

sazonov commented Feb 5, 2024

@Barry-RG those are critical fixes.

Copy link
Contributor

@Barry-RG Barry-RG left a comment

Choose a reason for hiding this comment

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

Thanks @Chris3LC and @sazonov for looking at this

@Barry-RG Barry-RG merged commit 7a180c8 into flyway:main Feb 5, 2024
1 check passed
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