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

Cockroach db compatibility #975

Closed
wants to merge 2 commits into from

Conversation

ruzkant
Copy link
Contributor

@ruzkant ruzkant commented Apr 8, 2018

[UPDATED]
This branch will attempt to add cockroachdb support to flowable.

I would like to get some feedback on preferences in regards to the implementation options.

Cockroachdb's core strategy seems to be to focus on being postgres compatible for ORM's and other tools. Although they may in future start supporting tools specifically so that cockroach's unique features can be better exploited.

I have run some tests with manually altered schema and this works, but automated schema upgrades present some challenges especially around transactions.

Implementation options
1. Create a new database in all places it is required, new scripts, new properties.
This approach is challenging for the following reasons:
a. The JDBC driver returns product as postgresql and even the versions are postgres versions. The recommended way to detect cockroach over postgres is to run the following query: select version() from which CockroachDB can be extracted.

The posthres driver hardcodes product name, but could retrieve it from a handshake parameters called crdb_version as used by other cockroach tools like pgasync. I tested making this change to the driver, it's simple and works, but I am not sure if a pull request would be accepted for this, even though it seems more correct than hardcoding. I suspect it would break tools that work with the driver currently (it sounds like hibernate works to some extent).

b. Liquibase might need a custom extension as there is none yet. This is not particularly hard to do and it has the concept of priority when overriding databases. Need is maybe a strong word, because so far I have been able to make changes to changesets that do work on the postgres implementation.

c. More work is required to keep schemas up to date, especially the non-liquibase ones.

2. Try to align postgres and cockroachdb implementations, to be compatible with each other.
This approach is challenging for its own reasons.
a. Foreign keys require indexes but you can't create them in the same transaction, not without postgres incompatible syntax. One option is to execute foreign keys in a second transaction which is not quite ideal (this approach is part of my initial commits). I opened an issue at cockroach here: cockroachdb/cockroach#24578
So this may be resolved in future.

b. Other such issues may arise in future, and one the differentiation then becomes a bit more haphazard.

c. More care needs to go into postgres scripts to make sure they don't disobey cockroach semantics.

Summary of issues found
I opened issues at cockroach for the following:

  1. A bug when calling jdbc metadata and then doing create table and insert queries in ddl scripts, sets off a sequence of events where it hangs.
    Postgresql JDBC driver hangs with certain sequence of queries cockroachdb/cockroach#24578

This requires starting a fresh transaction right before ddl scripts are run.

  1. The issue with foreign keys and corresponding indexes not being allowed in the same transaction has had some discussion here where it seems it might be resolvable in a future update: sql: can't create table, index and foreign key in the same transaction cockroachdb/cockroach#24626

Comments

  • there is some advantage to making the scripts equal for postgres and cockroach, one could have a postgres backup database in the event of some significant cockroachdb issue. I haven't tested it, but cockroach dumps claim to be loadable in other db's.
  • I am not particularly happy with executing the sql ddl scripts in two transactions.
  • With liquibase the transaction issue is less of a concern as each changeset seems to execute in a transaction.
  • Moving the sql based migrations to liquibase is probably also an option, if that is a longer term goal for flowable.
  • One can wait for some of the bugs to be resolved before taking this further

@ruzkant
Copy link
Contributor Author

ruzkant commented Apr 11, 2018

I ran into another issue with liquibase where it is unable to use the jdbc metadata to check for existing indexes. I notice quite a few open items on cockroachdb github related to supporting different functions. The one for the indexes is related to:
cockroachdb/cockroach#16971
cockroachdb/cockroach#16491
cockroachdb/cockroach#10495

It feels like the wild west out there, so I think I am going to close for now, I think quite a few issues will be solved in the near future, and for now will make do with static scripts to load the db schema.

@ruzkant ruzkant closed this Apr 11, 2018
@BramGruneir
Copy link

@ruzkant Please let us know what issues you require and we can prioritize them. Starting in our next release (as in what we're coding now), we're are putting a much bigger focus on external tool compatibility.

@ruzkant
Copy link
Contributor Author

ruzkant commented Apr 11, 2018

@BramGruneir I'll open up an item, but in short a lot of it has to do with the jdbc driver's metadata. You can literally go to the source and see if all the queries work:
https://github.com/pgjdbc/pgjdbc/blob/7a586b6e492e8911a928d50113a68569981fa731/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java

The one I was having trouble with was retrieving indexes, which is related to _pg_expandarray (but I am not sure if the rest of the query will work once moved there. Another one is related to getting key words which is string_agg function issue for which there is a related open item.

@BramGruneir
Copy link

Thanks! I'm actively tackling _pg_expandarray right now.

I'll take a look through all of those other queries.

Feel free to just comment on those already open issues. It will greatly help our prioritization process.

@ruzkant
Copy link
Contributor Author

ruzkant commented Apr 14, 2018

I am still testing cockroachdb for my own use, and built a simple compatibility layer, not suitable for general consumption. A layer of indirection solves every problem in computer science :)

I proxy the postgres driver, connection and statement classes, and detect and reorder ddl statements, and run them in appropriate transactions in the background. Introduce the odd index. This at least gets the apps started and works without any modifications to the current sql scripts or liquibase changelogs.

Just ran into correlated subquery issue on task application, and see correlated subquery support is not in 2.0 yet. I expect this to be a very difficult issue to get around if its use is prevalent, which I am not sure if it is.
cockroachdb/cockroach#24684

@ruzkant
Copy link
Contributor Author

ruzkant commented Dec 16, 2019

For anyone stumbling onto this... support for cockroachdb officially added in 6.4.2, yay!

#1716

@BramGruneir
Copy link

BramGruneir commented Dec 16, 2019 via email

@paulhh
Copy link
Contributor

paulhh commented Dec 16, 2019

Thanks to Cockroach Labs actively working to get it working with Flowable. Nice piece of collaboration.

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