JGroups JDBC_PING2 does not work with MSSQL Server

[thomasdarimont]

I tried to use JDBC_PING2 with MSSQL server with the following config:

<JDBC_PING2 datasource_injecter_class="com.github.thomasdarimont.jgroups.tools.CurrentDataSourceInjector"
                        register_shutdown_hook="true"
                        return_entire_cache="false"
                        remove_all_data_on_view_change="true"
                        
                        initialize_sql="CREATE TABLE jgroups (address varchar(200) NOT NULL, name varchar(200), cluster varchar(200) NOT NULL, ip varchar(200) NOT NULL, coord BIT, PRIMARY KEY (address) )"
			
                        stack.combine="REPLACE"
                        stack.position="MPING"/>
        </stack>

When I try to run that against MSSQL Server I see the following error:

2024-12-04 17:07:20,785 ERROR [org.jgroups.protocols.JDBC_PING2] (ForkJoinPool.commonPool-worker-1) neumann-20150: failed reading from the DB: com.microsoft.sqlserver.jdbc.SQLServerException: Invalid object name 'jgroups'.
	at com.microsoft.sqlserver.jdbc.SQLServerException.makeFromDatabaseError(SQLServerException.java:270)
...

and the jgroups table is not generated.

Turns out, JDBC_PING2 currently (as of JGroups 5.3.13) uses the following SQL to create the table:
https://github.com/belaban/JGroups/blob/master/src/org/jgroups/protocols/JDBC_PING2.java#L49

   protected String initialize_sql="CREATE TABLE jgroups (address varchar(200) NOT NULL, " +
      "name varchar(200), " +
      "cluster varchar(200) NOT NULL, " +
      "ip varchar(200) NOT NULL, " +
      "coord boolean, " +
      "PRIMARY KEY (address) )";

Unfortunately, MSSQL server does not support the column type boolean. Changing boolean to bit by overriding initialize_sql fixed the problem:

...
    <jgroups>
        <stack name="jdbc-ping-tcp" extends="tcp">
            <!-- for datasource_injecter_class see: https://github.com/keycloak/keycloak/discussions/29205#discussioncomment-9957585 -->
            <!-- for JDBC_PING2 DB Schema see: https://github.com/belaban/JGroups/blob/jgroups-5.3.13.Final/src/org/jgroups/protocols/JDBC_PING2.java#L49 -->
            <JDBC_PING2 datasource_injecter_class="com.github.thomasdarimont.jgroups.tools.CurrentDataSourceInjector"
                        register_shutdown_hook="true"
                        return_entire_cache="false"
                        remove_all_data_on_view_change="true"
                        
                        initialize_sql="CREATE TABLE jgroups (address varchar(200) NOT NULL, name varchar(200), cluster varchar(200) NOT NULL, ip varchar(200) NOT NULL, coord BIT, PRIMARY KEY (address) )"
			
                        stack.combine="REPLACE"
                        stack.position="MPING"/>
        </stack>
    </jgroups>
...

[belaban]

Hi @thomasdarimont is this solution sufficient for you? @ryanemerson: AIUI, you created templates for different DBs, is this something you can do in KC? [1] shows {{boolean}} to be a standard SQL type, so I don't really want to change this...
Thoughts?
[1] https://en.wikipedia.org/wiki/SQL

[ryanemerson]

Keycloak uses a combination of Liquibase and JPA to manage access to the database, so the types used for specific DBs in the table schema are handled for us by Liquibase and abstracted away by JPA/JDBC drivers when we perform operations.

The only thing we had to configure on the JDBC_PING2 side of things was the SQL queries as we modified the column names as the default "cluster" is not compatible with Oracle DB: https://github.com/keycloak/keycloak/blob/d7e5319f70b159a4c3482665d11749f50573f45a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/storage/infinispan/CacheManagerFactory.java#L442-L455

[belaban]

@ryanemerson no changes required from JGroups then?

[ryanemerson]

I think you're good. The only change to the defaults that would work for all DBs (at least within the Keycloak/Infinispan support range) is to change the column name from "cluster" to some variation in order to support Oracle DB. However, given that this would break backwards-compatibility, I don't think it's a change worth making, maybe just document?

[belaban]

OK, good to know, thx