[java-plugin-storage-jdbc] 03/03: OSJ-342 Investigate Strategies to end of life our use of Hibernate in V5

Rod Widdowson rdw at steadingsoftware.com
Tue May 24 16:02:30 UTC 2022


This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch main
in repository java-plugin-storage-jdbc.

View the commit online:
http://git.shibboleth.net/view/?p=java-plugin-storage-jdbc.git;a=commit;h=ca405d4cd5ffc032b881a20980ea06cebaa42224

commit ca405d4cd5ffc032b881a20980ea06cebaa42224
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Mon May 23 17:15:25 2022 +0100

    OSJ-342 Investigate Strategies to end of life our use of Hibernate in V5
    
    https://shibboleth.atlassian.net/browse/OSJ-342
    
    Add an optional ReadWrite log and, if it is present, use it to serialize
    access to the database (shared for reads, exclusive for everything else)
---
 .../storage/jdbc/impl/JDBCStorageService.java      | 122 ++++++++++++++++-----
 .../storage/jdbc/impl/JDBCStorageServiceTest.java  |   1 +
 2 files changed, 94 insertions(+), 29 deletions(-)

diff --git a/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java b/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java
index 8ce8f2a..df2eafc 100644
--- a/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java
+++ b/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java
@@ -29,6 +29,9 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.TimerTask;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -48,6 +51,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.annotation.constraint.Positive;
 import net.shibboleth.utilities.java.support.collection.Pair;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
@@ -120,6 +124,9 @@ public final class JDBCStorageService extends AbstractStorageService implements
     /** What transaction isolation do we want? */
     private int transactionIsolation = Connection.TRANSACTION_SERIALIZABLE;
     
+    /** If non-null we doing local locking. */
+    private ReadWriteLock readWriteLock;
+
     /** Error messages that signal a transaction should be retried. */
     @Nonnull @NonnullElements private Collection<String> retryableErrors = Collections.emptyList();
 
@@ -227,6 +234,18 @@ public final class JDBCStorageService extends AbstractStorageService implements
         }
     }
 
+    /** Will we do thread level locking or delegate to the Database?
+     * @param what do we want to lock locally?
+     */
+    public void setLocalLocking(final boolean what) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        if (what) {
+            readWriteLock = new ReentrantReadWriteLock(true);
+        } else {
+            readWriteLock = null;
+        }
+    }
+
     /** Set the parameter that will be passed to {@link Connection#setTransactionIsolation(int)}.
      * @param what the value to set
      */
@@ -391,7 +410,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
      */
     @Nonnull @NonnullElements protected List<String> readContexts() throws IOException {
         final List<String> result = new ArrayList<>();
-        try (final Connection connection = getConnection(true)) {
+        try (final ConnectionWithLock connection= new ConnectionWithLock(true, false)) {
             log.trace("ReadContexts:: ", readContextsSQL);
             final PreparedStatement query = connection.prepareStatement(readContextsSQL);
             final ResultSet results = query.executeQuery();
@@ -416,7 +435,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
      */
     @Nonnull @NonnullElements protected List<?> readAll() throws IOException {
         final List<JDBCStorageRecord<?>> result = new ArrayList<>();
-        try (final Connection connection = getConnection(true)) {
+        try (final ConnectionWithLock connection = new ConnectionWithLock(true, false)) {
             log.trace("ReadAll:: '{}' ", readAllSQL);
             final PreparedStatement query = connection.prepareStatement(readAllSQL);
             final ResultSet results = query.executeQuery();
@@ -451,7 +470,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
         final List<JDBCStorageRecord<?>> result = new ArrayList<>();
         Constraint.isNotEmpty(Constraint.isNotNull(context, "ReadAll(String): context must not be null"),
                               "ReadAll(String): context must not be empty");
-        try (final Connection connection = getConnection(true)) {
+        try (final ConnectionWithLock connection = new ConnectionWithLock(true, false)) {
             log.trace("ReadAll:: '{}' 1: '{}'  ", readAllByContextSQL, context);
             final PreparedStatement query = connection.prepareStatement(readAllByContextSQL);
             query.setString(1, context);
@@ -483,7 +502,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
                               "create: value must not be empty");
         int retries = transactionRetry;
         while(true) {
-            try (final Connection connection = getConnection(false)) {
+            try (final ConnectionWithLock connection = new ConnectionWithLock(false, true)) {
                 log.trace("Create [Query]:: '{}'::  1: '{}' ; 2: '{}'", preCreateQuerySQL, context, key);
                 // Does it exist?
                 // If not insert
@@ -579,7 +598,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
 
         int retries = transactionRetry;
         while(true) {
-            try (final Connection connection = getConnection(true)) {
+            try (final ConnectionWithLock connection = new ConnectionWithLock(true, false)) {
                 final PreparedStatement stmnt = connection.prepareStatement(readRecordSQL);
                 log.trace("Read:: '{}' 1: '{}' ; 2: '{}'", readRecordSQL, context, key);
                 stmnt.setString(1, context);
@@ -686,7 +705,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
                               "update: value must not be empty");
         int retries = transactionRetry;
         while (true) {
-            try (Connection connection = getConnection(false)) {
+            try (ConnectionWithLock connection = new ConnectionWithLock(false, true)) {
                 final PreparedStatement selectStmnt = connection.prepareStatement(preUpdateQuerySQL);
                 log.trace("Update [Query]:: '{}'  1: '{}' ; 2: '{}'", preUpdateQuerySQL, context, key);
 
@@ -782,7 +801,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
         Constraint.isTrue(version == null || version > 0, "delete: version should be null of > 0");
         int retries = transactionRetry;
         while (true) {
-            try (Connection connection = getConnection(false)) {
+            try (ConnectionWithLock connection= new ConnectionWithLock(false, true)) {
                 final PreparedStatement selectStmnt = connection.prepareStatement(preDeleteQuerySQL);
                 selectStmnt.setString(1, context);
                 selectStmnt.setString(2, key);
@@ -838,7 +857,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
         Constraint.isNotNull(expiration, "expiration: context must not be null");
         int retries = transactionRetry;
         while (true) {
-            try (Connection connection = getConnection(false)) {
+            try (ConnectionWithLock connection = new ConnectionWithLock(false, true)) {
                 final PreparedStatement updateStmnt = connection.prepareStatement(deleteByExpiredSQL);
                 updateStmnt.setLong(1, expiration);
                 log.trace("DeleteByExpired:: '{}':  1: '{}' ;", deleteByExpiredSQL, expiration);
@@ -874,7 +893,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
                               "reap: context must not be empty");
         int retries = transactionRetry;
         while (true) {
-            try (Connection connection = getConnection(true)) {
+            try (ConnectionWithLock connection = new ConnectionWithLock(true, true)) {
                 final PreparedStatement updateStmnt = connection.prepareStatement(deleteByContextExpiredSQL);
                 updateStmnt.setString(1, context);
                 final Long expires = System.currentTimeMillis();
@@ -912,7 +931,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
                               "updateContextExpiration: context must not be empty");
         int retries = transactionRetry;
         while (true) {
-            try (Connection connection = getConnection(true)) {
+            try (ConnectionWithLock connection = new ConnectionWithLock(true, true)) {
                 final PreparedStatement updateStmnt = connection.prepareStatement(updateExpiresByContextSQL);
                 setExpires(updateStmnt, 1, expires);
                 updateStmnt.setString(2, context);
@@ -951,7 +970,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
                               "deleteContext: context must not be empty");
         int retries = transactionRetry;
         while (true) {
-            try (Connection connection = getConnection(true)) {
+            try (ConnectionWithLock connection = new ConnectionWithLock(true, true)) {
                 final PreparedStatement updateStmnt = connection.prepareStatement(deleteByContextSQL);
                 updateStmnt.setString(1, context);
                 log.trace("UpdateContextExpiration:: '{}':  1: '{}'", deleteByContextSQL, context);
@@ -981,24 +1000,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
             }
         }
     }
-    
-    /**
-     * Obtain a connection from the data source.
-     * 
-     * <p>The caller must close the connection.</p>
-     * 
-     * @param autoCommit auto-commit setting to apply to the connection
-     * 
-     * @return a fresh connection
-     * @throws SQLException if an error occurs
-     */
-    @Nonnull private Connection getConnection(final boolean autoCommit) throws SQLException {
-        final Connection conn = dataSource.getConnection();
-        conn.setAutoCommit(autoCommit);
-        conn.setTransactionIsolation(transactionIsolation);
-        return conn;
-    }
-    
+
     /** Return the value of expires in the supplied column of the supplied {@link ResultSet}.
      * @param results the results whose current row we want to inspect
      * @param columm the column
@@ -1057,4 +1059,66 @@ public final class JDBCStorageService extends AbstractStorageService implements
             }
         };
     }
+
+    /** A Class to encapsulate a {@link Connection} protected by an optional
+     * read/write lock.
+     * Because the class implements {@link AutoCloseable} the unlock can "just happen"
+     */
+    private class ConnectionWithLock implements AutoCloseable {
+
+        /** The connection we set up. */
+        @Nonnull final Connection connection;
+
+        /** The lock we may or may not have set up. */
+        @Nullable final Lock threadLock;
+
+        /**
+         * @param autoCommit
+         * @param writeLock
+         * @throws SQLException
+         */
+        public ConnectionWithLock(final boolean autoCommit, final boolean writeLock) throws SQLException {
+            connection = dataSource.getConnection();
+            connection.setAutoCommit(autoCommit);
+            connection.setTransactionIsolation(transactionIsolation);
+            if (readWriteLock != null) {
+                if (writeLock) {
+                    threadLock = readWriteLock.writeLock();
+                } else {
+                    threadLock = readWriteLock.readLock();
+                }
+                threadLock.lock();
+            } else {
+                threadLock = null;
+            }
+        }
+
+	/** Delegated operation to the encapsulated {@link Connection}.
+         * @param Sql what to prepare
+         * @return what the encapsulated {@link Connection} returns
+         * @throws SQLException if encapsulated {@link Connection} does
+         */
+        public PreparedStatement prepareStatement(final String Sql) throws SQLException {
+            return connection.prepareStatement(Sql);
+        }
+
+        /** Delegated operation to the encapsulated {@link Connection}.
+         * @throws SQLException if encapsulated {@link Connection} does
+         */
+        public void commit() throws SQLException {
+            connection.commit();
+        }
+
+        @Override
+        public void close()  {
+            try {
+                connection.close();
+            } catch (SQLException e) {
+                log.error("Auto close failed", e);
+            }
+            if (threadLock != null) {
+                threadLock.unlock();
+            }
+        }
+    }
 }
diff --git a/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java b/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java
index 92e148f..5423b3c 100644
--- a/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java
+++ b/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java
@@ -132,6 +132,7 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
             storageService.setCleanupInterval(Duration.ofSeconds(5));
             storageService.setTransactionRetry(12);
             storageService.setRetryableErrors(List.of("40001"));
+            storageService.setLocalLocking(true);
         } catch (final SQLException | ClassNotFoundException e) {
             throw new ComponentInitializationException(e);
         }

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list