diff options
6 files changed, 30 insertions, 74 deletions
diff --git a/feature-distributed-locking/src/main/feature/config/feature-distributed-locking.properties b/feature-distributed-locking/src/main/feature/config/feature-distributed-locking.properties index c609c8f9..9acea3c0 100644 --- a/feature-distributed-locking/src/main/feature/config/feature-distributed-locking.properties +++ b/feature-distributed-locking/src/main/feature/config/feature-distributed-locking.properties @@ -19,7 +19,12 @@ ### #Database properties -javax.persistence.jdbc.driver= org.mariadb.jdbc.Driver +javax.persistence.jdbc.driver=org.mariadb.jdbc.Driver javax.persistence.jdbc.url=jdbc:mariadb://${env:SQL_HOST}:3306/pooling javax.persistence.jdbc.user=${env:SQL_USER} javax.persistence.jdbc.password=${env:SQL_PASSWORD} + +# default property values are commented out +#distributed.locking.expire.check.seconds=900 +#distributed.locking.retry.seconds=60 +#distributed.locking.max.retries=2 diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockManager.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockManager.java index 523c0d93..7ee786b9 100644 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockManager.java +++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockManager.java @@ -24,6 +24,7 @@ import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.SQLTransientException; import java.util.HashSet; import java.util.Map; import java.util.Properties; @@ -77,8 +78,7 @@ import org.slf4j.LoggerFactory; * instance.</li> * </dl> */ -public class DistributedLockManager - implements PolicyResourceLockManager, PolicyEngineFeatureApi { +public class DistributedLockManager implements PolicyResourceLockManager, PolicyEngineFeatureApi { private static final Logger logger = LoggerFactory.getLogger(DistributedLockManager.class); @@ -396,6 +396,8 @@ public class DistributedLockManager * Distributed Lock implementation. */ public static class DistributedLock extends LockImpl { + private static final String SQL_FAILED_MSG = "request failed for lock: {}"; + private static final long serialVersionUID = 1L; /** @@ -702,9 +704,9 @@ public class DistributedLockManager req.run(); } catch (SQLException e) { - logger.warn("request failed for lock: {}", this, e); + logger.warn(SQL_FAILED_MSG, this, e); - if (feature.featProps.isTransient(e.getErrorCode())) { + if (e.getCause() instanceof SQLTransientException) { // retry the request a little later rescheduleRequest(req); } else { @@ -712,7 +714,7 @@ public class DistributedLockManager } } catch (RuntimeException e) { - logger.warn("request failed for lock: {}", this, e); + logger.warn(SQL_FAILED_MSG, this, e); removeFromMap(); } } diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockProperties.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockProperties.java index f470c8e2..fff19447 100644 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockProperties.java +++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockProperties.java @@ -20,10 +20,7 @@ package org.onap.policy.distributed.locking; -import java.util.Collections; -import java.util.HashSet; import java.util.Properties; -import java.util.Set; import lombok.Getter; import lombok.Setter; import org.onap.policy.common.utils.properties.BeanConfigurator; @@ -40,7 +37,6 @@ public class DistributedLockProperties { public static final String DB_URL = "javax.persistence.jdbc.url"; public static final String DB_USER = "javax.persistence.jdbc.user"; public static final String DB_PASS = "javax.persistence.jdbc.password"; - public static final String TRANSIENT_ERROR_CODES = PREFIX + "transient.error.codes"; public static final String EXPIRE_CHECK_SEC = PREFIX + "expire.check.seconds"; public static final String RETRY_SEC = PREFIX + "retry.seconds"; public static final String MAX_RETRIES = PREFIX + "max.retries"; @@ -70,16 +66,6 @@ public class DistributedLockProperties { private String dbPwd; /** - * Vendor-specific error codes that are "transient", meaning they may go away if the - * command is repeated (e.g., connection issue), as opposed to something like a syntax - * error or a duplicate key. - */ - @Property(name = TRANSIENT_ERROR_CODES) - private String errorCodeStrings; - - private final Set<Integer> transientErrorCodes; - - /** * Time, in seconds, to wait between checks for expired locks. */ @Property(name = EXPIRE_CHECK_SEC, defaultValue = "900") @@ -105,32 +91,5 @@ public class DistributedLockProperties { */ public DistributedLockProperties(Properties props) throws PropertyException { new BeanConfigurator().configureFromProperties(this, props); - - Set<Integer> set = new HashSet<>(); - for (String text : errorCodeStrings.split(",")) { - text = text.trim(); - if (text.isEmpty()) { - continue; - } - - try { - set.add(Integer.valueOf(text)); - - } catch (NumberFormatException e) { - throw new PropertyException(TRANSIENT_ERROR_CODES, "errorCodeStrings", e); - } - } - - transientErrorCodes = Collections.unmodifiableSet(set); - } - - /** - * Determines if an error is transient. - * - * @param errorCode error code to check - * @return {@code true} if the error is transient, {@code false} otherwise - */ - public boolean isTransient(int errorCode) { - return transientErrorCodes.contains(errorCode); } } diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockManagerTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockManagerTest.java index 59b56224..0ba92c51 100644 --- a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockManagerTest.java +++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockManagerTest.java @@ -48,6 +48,7 @@ import java.sql.DriverManager; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.SQLTransientException; import java.util.ArrayList; import java.util.List; import java.util.Properties; @@ -100,8 +101,8 @@ public class DistributedLockManagerTest { private static final int HOLD_SEC2 = 120; private static final int MAX_THREADS = 5; private static final int MAX_LOOPS = 100; - private static final int TRANSIENT = 500; - private static final int PERMANENT = 600; + private static final boolean TRANSIENT = true; + private static final boolean PERMANENT = false; // number of execute() calls before the first lock attempt private static final int PRE_LOCK_EXECS = 1; @@ -1673,15 +1674,15 @@ public class DistributedLockManagerTest { * Feature whose data source all throws exceptions. */ private class InvalidDbLockingFeature extends MyLockingFeature { - private int errcode; + private boolean isTransient; private boolean freeLock = false; - public InvalidDbLockingFeature(int errcode) { + public InvalidDbLockingFeature(boolean isTransient) { // pass "false" because we have to set the error code BEFORE calling // afterStart() super(false); - this.errcode = errcode; + this.isTransient = isTransient; this.beforeCreateLockManager(engine, new Properties()); this.afterStart(engine); @@ -1695,13 +1696,22 @@ public class DistributedLockManagerTest { lock.free(); } - throw new SQLException(EXPECTED_EXCEPTION, "", errcode); + throw makeEx(); }); - doThrow(new SQLException(EXPECTED_EXCEPTION, "", errcode)).when(datasrc).close(); + doThrow(makeEx()).when(datasrc).close(); return datasrc; } + + private SQLException makeEx() { + if (isTransient) { + return new SQLException(new SQLTransientException(EXPECTED_EXCEPTION)); + + } else { + return new SQLException(EXPECTED_EXCEPTION); + } + } } /** diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockPropertiesTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockPropertiesTest.java index 5f76d657..5abca889 100644 --- a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockPropertiesTest.java +++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockPropertiesTest.java @@ -20,13 +20,9 @@ package org.onap.policy.distributed.locking; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import java.util.Properties; -import java.util.TreeSet; import org.junit.Before; import org.junit.Test; import org.onap.policy.common.utils.properties.exception.PropertyException; @@ -46,7 +42,6 @@ public class DistributedLockPropertiesTest { props.setProperty(DistributedLockProperties.DB_URL, "my url"); props.setProperty(DistributedLockProperties.DB_USER, "my user"); props.setProperty(DistributedLockProperties.DB_PASS, "my pass"); - props.setProperty(DistributedLockProperties.TRANSIENT_ERROR_CODES, "10,-20,,,30"); props.setProperty(DistributedLockProperties.EXPIRE_CHECK_SEC, "100"); props.setProperty(DistributedLockProperties.RETRY_SEC, "200"); props.setProperty(DistributedLockProperties.MAX_RETRIES, "300"); @@ -60,22 +55,8 @@ public class DistributedLockPropertiesTest { assertEquals("my url", dlp.getDbUrl()); assertEquals("my user", dlp.getDbUser()); assertEquals("my pass", dlp.getDbPwd()); - assertEquals("10,-20,,,30", dlp.getErrorCodeStrings()); - assertEquals("[-20, 10, 30]", new TreeSet<>(dlp.getTransientErrorCodes()).toString()); assertEquals(100, dlp.getExpireCheckSec()); assertEquals(200, dlp.getRetrySec()); assertEquals(300, dlp.getMaxRetries()); - - assertTrue(dlp.isTransient(10)); - assertTrue(dlp.isTransient(-20)); - assertTrue(dlp.isTransient(30)); - - assertFalse(dlp.isTransient(-10)); - - // invalid value - props.setProperty(DistributedLockProperties.TRANSIENT_ERROR_CODES, "10,abc,30"); - - assertThatThrownBy(() -> new DistributedLockProperties(props)).isInstanceOf(PropertyException.class) - .hasMessageContaining(DistributedLockProperties.TRANSIENT_ERROR_CODES); } } diff --git a/feature-distributed-locking/src/test/resources/feature-distributed-locking.properties b/feature-distributed-locking/src/test/resources/feature-distributed-locking.properties index 061cc608..0fca3c04 100644 --- a/feature-distributed-locking/src/test/resources/feature-distributed-locking.properties +++ b/feature-distributed-locking/src/test/resources/feature-distributed-locking.properties @@ -23,7 +23,6 @@ javax.persistence.jdbc.url=jdbc:h2:mem:pooling javax.persistence.jdbc.user=user javax.persistence.jdbc.password=password -distributed.locking.transient.error.codes=500 distributed.locking.expire.check.seconds=900 distributed.locking.retry.seconds=60 distributed.locking.max.retries=2 |