From 604db9ee7e01a91ec41d95f67a37a2d21afaa74c Mon Sep 17 00:00:00 2001 From: Martyn Taylor Date: Tue, 13 Jun 2017 15:02:31 +0100 Subject: [PATCH] ARTEMIS-1204 Replace open sync with AtomicBoolean The JDBCSequentialFile blocks on the writeLock when opening. There is no need to block here, in fact it may cause issues when opening and syncing concurrently. Instead an AtomicBoolean is enough to prevent the file from being reloaded. --- .../jdbc/store/file/JDBCSequentialFile.java | 32 ++++++++++--------- .../file/JDBCSequentialFileFactoryDriver.java | 2 +- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFile.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFile.java index 018000dfae..9d1be1fdec 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFile.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFile.java @@ -23,6 +23,7 @@ import java.sql.SQLException; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.activemq.artemis.api.core.ActiveMQBuffer; import org.apache.activemq.artemis.api.core.ActiveMQBuffers; @@ -43,9 +44,9 @@ public class JDBCSequentialFile implements SequentialFile { private final String extension; - private boolean isOpen = false; + private AtomicBoolean isOpen = new AtomicBoolean(false); - private boolean isLoaded = false; + private AtomicBoolean isLoaded = new AtomicBoolean(false); private long id = -1; @@ -83,12 +84,12 @@ public class JDBCSequentialFile implements SequentialFile { @Override public boolean isOpen() { - return isOpen; + return isOpen.get(); } @Override public boolean exists() { - if (isLoaded) return true; + if (isLoaded.get()) return true; try { return fileFactory.listFiles(extension).contains(filename); } catch (Exception e) { @@ -100,21 +101,20 @@ public class JDBCSequentialFile implements SequentialFile { @Override public void open() throws Exception { - load(); - isOpen = true; + isOpen.compareAndSet(false, load()); } - private void load() { + private boolean load() { try { - synchronized (writeLock) { - if (!isLoaded) { - dbDriver.openFile(this); - isLoaded = true; - } + if (isLoaded.compareAndSet(false, true)) { + dbDriver.openFile(this); } + return true; } catch (SQLException e) { + isLoaded.set(false); fileFactory.onIOError(e, "Error attempting to open JDBC file.", this); } + return false; } @Override @@ -146,8 +146,9 @@ public class JDBCSequentialFile implements SequentialFile { public void delete() throws IOException, InterruptedException, ActiveMQException { try { synchronized (writeLock) { - load(); - dbDriver.deleteFile(this); + if (load()) { + dbDriver.deleteFile(this); + } } } catch (SQLException e) { fileFactory.onIOError(e, "Error deleting JDBC file.", this); @@ -156,6 +157,7 @@ public class JDBCSequentialFile implements SequentialFile { private synchronized int internalWrite(byte[] data, IOCallback callback) { try { + open(); synchronized (writeLock) { int noBytes = dbDriver.writeToFile(this, data); seek(noBytes); @@ -281,7 +283,7 @@ public class JDBCSequentialFile implements SequentialFile { @Override public void close() throws Exception { - isOpen = false; + isOpen.set(false); sync(); fileFactory.sequentialFileClosed(this); } diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactoryDriver.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactoryDriver.java index 6cf54ffaa0..fdd7c764a4 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactoryDriver.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactoryDriver.java @@ -165,7 +165,7 @@ public class JDBCSequentialFileFactoryDriver extends AbstractJDBCDriver { if (blob != null) { file.setWritePosition(blob.length()); } else { - logger.warn("ERROR NO BLOB FOR FILE" + "File: " + file.getFileName() + " " + file.getId()); + logger.trace("No Blob found for file: " + file.getFileName() + " " + file.getId()); } } connection.commit();