libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

Ensure lock table has its primary key created, and re-enable the DBLockTest

Browse source 
Closes #15487
This commit is contained in:
Alexander Schwartz 2022-12-15 10:14:30 +01:00 committed by Hynek Mlnařík
parent 23ad6ccd0f
commit 6d0e112bf1
2 changed files with 45 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

48
model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/CustomLockService.java
View file

@ -24,11 +24,18 @@ import liquibase.exception.UnexpectedLiquibaseException;
import liquibase.executor.Executor;
import liquibase.executor.ExecutorService;
import liquibase.lockservice.StandardLockService;
import liquibase.snapshot.DatabaseSnapshot;
import liquibase.snapshot.InvalidExampleException;
import liquibase.snapshot.SnapshotControl;
import liquibase.snapshot.SnapshotGeneratorFactory;
import liquibase.statement.core.CreateDatabaseChangeLogLockTableStatement;
import liquibase.statement.core.DropTableStatement;
import liquibase.statement.core.InitializeDatabaseChangeLogLockTableStatement;
import liquibase.statement.core.LockDatabaseChangeLogStatement;
import liquibase.statement.core.RawSqlStatement;
import liquibase.structure.core.PrimaryKey;
import liquibase.structure.core.Schema;
import liquibase.structure.core.Table;
import org.jboss.logging.Logger;
import org.keycloak.common.util.Time;
import org.keycloak.common.util.reflections.Reflections;
@ -52,6 +59,41 @@ public class CustomLockService extends StandardLockService {
private static final Logger log = Logger.getLogger(CustomLockService.class);
@Override
protected boolean hasDatabaseChangeLogLockTable() throws DatabaseException {
boolean originalReturnValue = super.hasDatabaseChangeLogLockTable();
if (originalReturnValue && database.getConnection().getDatabaseProductName().equals("H2")) {
/* Liquibase only checks that the table exists. On the H2 database, creation of a table with a primary key is not atomic,
and the primary key might not be visible yet. The primary key would be needed to prevent inserting the data into the table
a second time. Inserting it a second time might lead to a failure when creating the primary key, which would then roll back
the creation of the table. Therefore, at least on the H2 database, checking for the primary key is essential.
An existing DATABASECHANGELOG might indicate that the insertion of data was completed previously.
Still, this isn't working with the DBLockTest which deletes only the DATABASECHANGELOGLOCK table.
See https://github.com/keycloak/keycloak/issues/15487 for more information.
*/
Table lockTable = (Table) new Table().setName(database.getDatabaseChangeLogLockTableName()).setSchema(
new Schema(database.getLiquibaseCatalogName(), database.getLiquibaseSchemaName()));
SnapshotGeneratorFactory instance = SnapshotGeneratorFactory.getInstance();
try {
DatabaseSnapshot snapshot = instance.createSnapshot(lockTable.getSchema().toCatalogAndSchema(), database,
new SnapshotControl(database, false, Table.class, PrimaryKey.class).setWarnIfObjectNotFound(false));
Table lockTableFromSnapshot = snapshot.get(lockTable);
if (lockTableFromSnapshot == null) {
throw new RuntimeException("DATABASECHANGELOGLOCK not found, although Liquibase claims it exists.");
} else if (lockTableFromSnapshot.getPrimaryKey() == null) {
log.warn("Primary key not found - table creation not complete yet.");
return false;
}
} catch (InvalidExampleException e) {
throw new RuntimeException(e);
}
}
return originalReturnValue;
}
@Override
public void init() throws DatabaseException {
Executor executor = Scope.getCurrentScope().getSingleton(ExecutorService.class).getExecutor(LiquibaseConstants.JDBC_EXECUTOR, database);
@ -65,7 +107,7 @@ public class CustomLockService extends StandardLockService {
executor.execute(new CreateDatabaseChangeLogLockTableStatement());
database.commit();
} catch (DatabaseException de) {
log.warn("Failed to create lock table. Maybe other transaction created in the meantime. Retrying...");
log.warn("Failed to create lock table. Maybe other transaction created in the meantime. Retrying...", de);
if (log.isTraceEnabled()) {
log.trace(de.getMessage(), de); //Log details at trace level
}
@ -97,7 +139,7 @@ public class CustomLockService extends StandardLockService {
}
} catch (DatabaseException de) {
log.warn("Failed to insert first record to the lock table. Maybe other transaction inserted in the meantime. Retrying...");
log.warn("Failed to insert first record to the lock table. Maybe other transaction inserted in the meantime. Retrying...", de);
if (log.isTraceEnabled()) {
log.trace(de.getMessage(), de); // Log details at trace level
}
@ -212,7 +254,7 @@ public class CustomLockService extends StandardLockService {
return true;
} catch (DatabaseException de) {
log.warn("Lock didn't yet acquired. Will possibly retry to acquire lock. Details: " + de.getMessage());
log.warn("Lock didn't yet acquired. Will possibly retry to acquire lock. Details: " + de.getMessage(), de);
if (log.isTraceEnabled()) {
log.debug(de.getMessage(), de);
}

3
testsuite/model/src/test/java/org/keycloak/testsuite/model/DBLockTest.java
View file

@ -23,7 +23,6 @@ import java.util.concurrent.atomic.AtomicInteger;
import org.jboss.logging.Logger;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
@ -108,7 +107,6 @@ public class DBLockTest extends KeycloakModelTest {
}
@Test
@Ignore("Ignored as it is unstable. Analysis pending in https://github.com/keycloak/keycloak/issues/15487")
public void testLockConcurrentlyGeneral() throws Exception {
inComittedTransaction(1, (session , i) -> {
testLockConcurrentlyInternal(session, DBLockProvider.Namespace.DATABASE);
@ -117,7 +115,6 @@ public class DBLockTest extends KeycloakModelTest {
}
@Test
@Ignore("Ignored as it is unstable. Analysis pending in https://github.com/keycloak/keycloak/issues/15487")
public void testLockConcurrentlyOffline() throws Exception {
inComittedTransaction(1, (session , i) -> {
testLockConcurrentlyInternal(session, DBLockProvider.Namespace.OFFLINE_SESSIONS);