From a8259e2a141ae7d0b9aedcb7b24638817b6e6ac8 Mon Sep 17 00:00:00 2001 From: Woonsan Ko Date: Fri, 26 Aug 2016 20:07:30 -0400 Subject: [PATCH 1/5] JCR-4006: When async write pool size is zero or smaller, don't write asynchronously. --- .../jackrabbit/aws/ext/ds/S3Backend.java | 37 +++-- .../jackrabbit/core/data/AbstractBackend.java | 140 ++++++++++++++++++ .../jackrabbit/core/data/FSBackend.java | 34 ++--- .../jackrabbit/core/data/InMemoryBackend.java | 22 +-- .../jackrabbit/vfs/ext/ds/VFSBackend.java | 101 +------------ .../jackrabbit/vfs/ext/ds/VFSDataStore.java | 2 +- 6 files changed, 181 insertions(+), 155 deletions(-) create mode 100644 jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java diff --git a/jackrabbit-aws-ext/src/main/java/org/apache/jackrabbit/aws/ext/ds/S3Backend.java b/jackrabbit-aws-ext/src/main/java/org/apache/jackrabbit/aws/ext/ds/S3Backend.java index 48b3ed17adc..1ac8ae0fd2d 100644 --- a/jackrabbit-aws-ext/src/main/java/org/apache/jackrabbit/aws/ext/ds/S3Backend.java +++ b/jackrabbit-aws-ext/src/main/java/org/apache/jackrabbit/aws/ext/ds/S3Backend.java @@ -36,11 +36,11 @@ import org.apache.jackrabbit.aws.ext.S3Constants; import org.apache.jackrabbit.aws.ext.S3RequestDecorator; import org.apache.jackrabbit.aws.ext.Utils; +import org.apache.jackrabbit.core.data.AbstractBackend; import org.apache.jackrabbit.core.data.AsyncTouchCallback; import org.apache.jackrabbit.core.data.AsyncTouchResult; import org.apache.jackrabbit.core.data.AsyncUploadCallback; import org.apache.jackrabbit.core.data.AsyncUploadResult; -import org.apache.jackrabbit.core.data.Backend; import org.apache.jackrabbit.core.data.CachingDataStore; import org.apache.jackrabbit.core.data.DataIdentifier; import org.apache.jackrabbit.core.data.DataStoreException; @@ -71,7 +71,7 @@ /** * A data store backend that stores data on Amazon S3. */ -public class S3Backend implements Backend { +public class S3Backend extends AbstractBackend { /** * Logger instance. @@ -86,13 +86,9 @@ public class S3Backend implements Backend { private TransferManager tmx; - private CachingDataStore store; - private Properties properties; private Date startTime; - - private ThreadPoolExecutor asyncWriteExecuter; private S3RequestDecorator s3ReqDecorator; @@ -101,8 +97,10 @@ public class S3Backend implements Backend { * aws.properties. It creates S3 bucket if it doesn't pre-exist in S3. */ @Override - public void init(CachingDataStore store, String homeDir, String config) + public void init(CachingDataStore cachingDataStore, String homeDir, String config) throws DataStoreException { + super.init(cachingDataStore, homeDir, config); + Properties initProps = null; //Check is configuration is already provided. That takes precedence //over config provided via file based config @@ -120,19 +118,19 @@ public void init(CachingDataStore store, String homeDir, String config) } this.properties = initProps; } - init(store, homeDir, initProps); + + init(cachingDataStore, homeDir, initProps); } - public void init(CachingDataStore store, String homeDir, Properties prop) + public void init(CachingDataStore cachingDataStore, String homeDir, Properties prop) throws DataStoreException { - ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); try { startTime = new Date(); Thread.currentThread().setContextClassLoader( getClass().getClassLoader()); LOG.debug("init"); - this.store = store; + setCachingDataStore(cachingDataStore); s3ReqDecorator = new S3RequestDecorator(prop); s3service = Utils.openService(prop); @@ -184,9 +182,8 @@ public void init(CachingDataStore store, String homeDir, Properties prop) asyncWritePoolSize = Integer.parseInt(maxConnsStr) - writeThreads; } - - asyncWriteExecuter = (ThreadPoolExecutor) Executors.newFixedThreadPool( - asyncWritePoolSize, new NamedThreadFactory("s3-write-worker")); + setAsyncWritePoolSize(asyncWritePoolSize); + String renameKeyProp = prop.getProperty(S3Constants.S3_RENAME_KEYS); boolean renameKeyBool = (renameKeyProp == null || "".equals(renameKeyProp)) ? false @@ -226,7 +223,7 @@ public void writeAsync(DataIdentifier identifier, File file, throw new IllegalArgumentException( "callback parameter cannot be null in asyncUpload"); } - asyncWriteExecuter.execute(new AsyncUploadJob(identifier, file, + getAsyncWriteExecutor().execute(new AsyncUploadJob(identifier, file, callback)); } @@ -327,7 +324,7 @@ public void touchAsync(final DataIdentifier identifier, Thread.currentThread().setContextClassLoader( getClass().getClassLoader()); - asyncWriteExecuter.execute(new Runnable() { + getAsyncWriteExecutor().execute(new Runnable() { @Override public void run() { try { @@ -537,13 +534,13 @@ public Set deleteAllOlderThan(long min) long lastModified = s3ObjSumm.getLastModified().getTime(); LOG.debug("Identifier [{}]'s lastModified = [{}]", identifier, lastModified); if (lastModified < min - && store.confirmDelete(identifier) + && getCachingDataStore().confirmDelete(identifier) // confirm once more that record's lastModified < min // order is important here && s3service.getObjectMetadata(bucket, s3ObjSumm.getKey()).getLastModified().getTime() < min) { - store.deleteFromCache(identifier); + getCachingDataStore().deleteFromCache(identifier); LOG.debug("add id [{}] to delete lists", s3ObjSumm.getKey()); deleteList.add(new DeleteObjectsRequest.KeyVersion( @@ -584,9 +581,9 @@ public Set deleteAllOlderThan(long min) } @Override - public void close() { + public void close() throws DataStoreException { + super.close(); // backend is closing. abort all mulitpart uploads from start. - asyncWriteExecuter.shutdownNow(); if(s3service.doesBucketExist(bucket)) { tmx.abortMultipartUploads(bucket, startTime); } diff --git a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java new file mode 100644 index 00000000000..4333c05630d --- /dev/null +++ b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java @@ -0,0 +1,140 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.core.data; + +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadPoolExecutor; + +import org.apache.jackrabbit.core.data.util.NamedThreadFactory; + +/** + * Abstract Backend which has a reference to the underlying {@link CachingDataStore} and is + * maintaining the lifecycle of the internal asynchronous write executor. + */ +public abstract class AbstractBackend implements Backend { + + /** + * {@link CachingDataStore} instance using this backend. + */ + private CachingDataStore cachingDataStore; + + /** + * The pool size of asynchronous write pooling executor. + */ + private int asyncWritePoolSize; + + /** + * Asynchronous write pooling executor. + */ + private Executor asyncWriteExecutor; + + /** + * Returns the pool size of the asynchronous write pool executor. + * @return the pool size of the asynchronous write pool executor + */ + public int getAsyncWritePoolSize() { + return asyncWritePoolSize; + } + + /** + * Sets the pool size of the asynchronous write pool executor. + * @param asyncWritePoolSize pool size of the async write pool executor + */ + public void setAsyncWritePoolSize(int asyncWritePoolSize) { + this.asyncWritePoolSize = asyncWritePoolSize; + } + + /** + * {@inheritDoc} + */ + @Override + public void init(CachingDataStore cachingDataStore, String homeDir, String config) throws DataStoreException { + this.cachingDataStore = cachingDataStore; + } + + /** + * {@inheritDoc} + */ + @Override + public void close() throws DataStoreException { + Executor asyncExecutor = getAsyncWriteExecutor(); + + if (asyncExecutor != null && asyncExecutor instanceof ExecutorService) { + ((ExecutorService) asyncExecutor).shutdownNow(); + } + } + + /** + * Returns the {@link CachingDataStore} instance using this backend. + * @return the {@link CachingDataStore} instance using this backend + */ + protected CachingDataStore getCachingDataStore() { + return cachingDataStore; + } + + /** + * Sets the {@link CachingDataStore} instance using this backend. + * @param cachingDataStore the {@link CachingDataStore} instance using this backend + */ + protected void setCachingDataStore(CachingDataStore cachingDataStore) { + this.cachingDataStore = cachingDataStore; + } + + /** + * Returns Executor used to execute asynchronous write or touch jobs. + * @return Executor used to execute asynchronous write or touch jobs + */ + protected Executor getAsyncWriteExecutor() { + if (asyncWriteExecutor == null) { + asyncWriteExecutor = createAsyncWriteExecutor(); + } + + return asyncWriteExecutor; + } + + /** + * Creates an {@link Executor}. + * This method is invoked during the initialization for asynchronous write/touch job executions. + * @return an {@link Executor} + */ + protected Executor createAsyncWriteExecutor() { + Executor asyncExecutor; + + if (getAsyncWritePoolSize() > 0) { + asyncExecutor = (ThreadPoolExecutor) Executors.newFixedThreadPool(getAsyncWritePoolSize(), + new NamedThreadFactory(getClass().getSimpleName() + "-write-worker")); + } else { + asyncExecutor = new ImmediateExecutor(); + } + + return asyncExecutor; + } + + /** + * This class implements {@link Executor} interface to run {@code command} right away, + * resulting in non-asynchronous mode executions. + */ + private class ImmediateExecutor implements Executor { + @Override + public void execute(Runnable command) { + command.run(); + } + } + +} diff --git a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java index d8f7e228ace..6b7b61a9b55 100644 --- a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java +++ b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java @@ -32,31 +32,24 @@ import java.util.List; import java.util.Properties; import java.util.Set; -import java.util.concurrent.Executors; -import java.util.concurrent.ThreadPoolExecutor; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; -import org.apache.jackrabbit.core.data.util.NamedThreadFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class FSBackend implements Backend { +public class FSBackend extends AbstractBackend { private Properties properties; private String fsPath; - private CachingDataStore store; - private String homeDir; private String config; File fsPathDir; - private ThreadPoolExecutor asyncWriteExecuter; - public static final String FS_BACKEND_PATH = "fsBackendPath"; /** @@ -70,8 +63,10 @@ public class FSBackend implements Backend { private static final int ACCESS_TIME_RESOLUTION = 2000; @Override - public void init(CachingDataStore store, String homeDir, String config) + public void init(CachingDataStore cachingDataStore, String homeDir, String config) throws DataStoreException { + super.init(cachingDataStore, homeDir, config); + Properties initProps = null; // Check is configuration is already provided. That takes precedence // over config provided via file based config @@ -92,13 +87,13 @@ public void init(CachingDataStore store, String homeDir, String config) } this.properties = initProps; } - init(store, homeDir, initProps); + init(cachingDataStore, homeDir, initProps); } public void init(CachingDataStore store, String homeDir, Properties prop) throws DataStoreException { - this.store = store; + setCachingDataStore(store); this.homeDir = homeDir; this.fsPath = prop.getProperty(FS_BACKEND_PATH); if (this.fsPath == null || "".equals(this.fsPath)) { @@ -117,9 +112,6 @@ public void init(CachingDataStore store, String homeDir, Properties prop) + fsPathDir.getAbsolutePath()); } } - asyncWriteExecuter = (ThreadPoolExecutor) Executors.newFixedThreadPool( - 10, new NamedThreadFactory("fs-write-worker")); - } @Override @@ -190,7 +182,7 @@ public void writeAsync(final DataIdentifier identifier, final File src, throw new IllegalArgumentException( "callback parameter cannot be null in asyncUpload"); } - asyncWriteExecuter.execute(new Runnable() { + getAsyncWriteExecutor().execute(new Runnable() { @Override public void run() { try { @@ -268,7 +260,7 @@ public void touchAsync(final DataIdentifier identifier, Thread.currentThread().setContextClassLoader( getClass().getClassLoader()); - asyncWriteExecuter.execute(new Runnable() { + getAsyncWriteExecutor().execute(new Runnable() { @Override public void run() { try { @@ -290,12 +282,6 @@ public void run() { } - @Override - public void close() throws DataStoreException { - asyncWriteExecuter.shutdownNow(); - - } - @Override public Set deleteAllOlderThan(long min) throws DataStoreException { @@ -457,8 +443,8 @@ private void deleteOlderRecursive(File file, long min, } if (lastModified < min) { DataIdentifier id = new DataIdentifier(file.getName()); - if (store.confirmDelete(id)) { - store.deleteFromCache(id); + if (getCachingDataStore().confirmDelete(id)) { + getCachingDataStore().deleteFromCache(id); if (LOG.isInfoEnabled()) { LOG.info("Deleting old file " + file.getAbsolutePath() + " modified: " diff --git a/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/InMemoryBackend.java b/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/InMemoryBackend.java index 5360a74258b..0cdd4aaa764 100644 --- a/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/InMemoryBackend.java +++ b/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/InMemoryBackend.java @@ -33,28 +33,20 @@ /** * An in-memory backend implementation used to speed up testing. */ -public class InMemoryBackend implements Backend { +public class InMemoryBackend extends AbstractBackend { private HashMap data = new HashMap(); private HashMap timeMap = new HashMap(); - - private CachingDataStore store; - + private Properties properties; @Override - public void init(CachingDataStore store, String homeDir, String config) + public void init(CachingDataStore cachingDataStore, String homeDir, String config) throws DataStoreException { + super.init(cachingDataStore, homeDir, config); // ignore log("init"); - this.store = store; - } - - @Override - public void close() { - // ignore - log("close"); } @Override @@ -110,9 +102,9 @@ public Set deleteAllOlderThan(final long min) throws DataStoreEx for (Map.Entry entry : timeMap.entrySet()) { DataIdentifier identifier = entry.getKey(); long timestamp = entry.getValue(); - if (timestamp < min && !store.isInUse(identifier) - && store.confirmDelete(identifier)) { - store.deleteFromCache(identifier); + if (timestamp < min && !getCachingDataStore().isInUse(identifier) + && getCachingDataStore().confirmDelete(identifier)) { + getCachingDataStore().deleteFromCache(identifier); tobeDeleted.add(identifier); } } diff --git a/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSBackend.java b/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSBackend.java index e233d7157b8..9261c4d8542 100644 --- a/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSBackend.java +++ b/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSBackend.java @@ -28,41 +28,33 @@ import java.util.List; import java.util.Set; import java.util.concurrent.Executor; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.concurrent.ThreadPoolExecutor; import org.apache.commons.io.IOUtils; import org.apache.commons.vfs2.FileObject; import org.apache.commons.vfs2.FileSystemException; import org.apache.commons.vfs2.FileType; +import org.apache.jackrabbit.core.data.AbstractBackend; import org.apache.jackrabbit.core.data.AsyncTouchCallback; import org.apache.jackrabbit.core.data.AsyncTouchResult; import org.apache.jackrabbit.core.data.AsyncUploadCallback; import org.apache.jackrabbit.core.data.AsyncUploadResult; -import org.apache.jackrabbit.core.data.Backend; import org.apache.jackrabbit.core.data.CachingDataStore; import org.apache.jackrabbit.core.data.DataIdentifier; import org.apache.jackrabbit.core.data.DataStoreException; -import org.apache.jackrabbit.core.data.util.NamedThreadFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * A data store backend that stores data on VFS file system. */ -public class VFSBackend implements Backend { +public class VFSBackend extends AbstractBackend { /** * Logger instance. */ private static final Logger LOG = LoggerFactory.getLogger(VFSBackend.class); - /** - * The default pool size of asynchronous write pooling executor. - */ - static final int DEFAULT_ASYNC_WRITE_POOL_SIZE = 10; - /** * The maximum last modified time resolution of the file system. */ @@ -78,26 +70,11 @@ public class VFSBackend implements Backend { */ private static final String TOUCH_FILE_NAME_SUFFIX = ".touch"; - /** - * {@link CachingDataStore} instance using this backend. - */ - private CachingDataStore store; - /** * VFS base folder object. */ private FileObject baseFolder; - /** - * The pool size of asynchronous write pooling executor. - */ - private int asyncWritePoolSize = DEFAULT_ASYNC_WRITE_POOL_SIZE; - - /** - * Asynchronous write pooling executor. - */ - private Executor asyncWriteExecutor; - /** * Whether or not a touch file is preferred to set/get the last modified timestamp for a file object * instead of setting/getting the last modified timestamp directly from the file object. @@ -108,35 +85,18 @@ public VFSBackend(FileObject baseFolder) { this.baseFolder = baseFolder; } - /** - * Returns the pool size of the async write pool executor. - * @return the pool size of the async write pool executor - */ - public int getAsyncWritePoolSize() { - return asyncWritePoolSize; - } - - /** - * Sets the pool size of the async write pool executor. - * @param asyncWritePoolSize pool size of the async write pool executor - */ - public void setAsyncWritePoolSize(int asyncWritePoolSize) { - this.asyncWritePoolSize = asyncWritePoolSize; - } - /** * {@inheritDoc} */ @Override - public void init(CachingDataStore store, String homeDir, String config) throws DataStoreException { - this.store = store; + public void init(CachingDataStore cachingDataStore, String homeDir, String config) throws DataStoreException { + super.init(cachingDataStore, homeDir, config); // When it's local file system, no need to use a separate touch file. if ("file".equals(baseFolder.getName().getScheme())) { touchFilePreferred = false; } - asyncWriteExecutor = createAsyncWriteExecutor(); } /** @@ -277,18 +237,6 @@ public void touchAsync(DataIdentifier identifier, long minModifiedDate, AsyncTou getAsyncWriteExecutor().execute(new AsyncTouchJob(identifier, minModifiedDate, callback)); } - /** - * {@inheritDoc} - */ - @Override - public void close() throws DataStoreException { - Executor asyncExecutor = getAsyncWriteExecutor(); - - if (asyncExecutor != null && asyncExecutor instanceof ExecutorService) { - ((ExecutorService) asyncExecutor).shutdownNow(); - } - } - /** * {@inheritDoc} */ @@ -455,32 +403,6 @@ protected FileObject getTouchFileObject(FileObject fileObject, boolean create) t } } - /** - * Creates a {@link ThreadPoolExecutor}. - * This method is invoked during the initialization for asynchronous write/touch job executions. - * @return a {@link ThreadPoolExecutor} - */ - protected Executor createAsyncWriteExecutor() { - Executor asyncExecutor; - - if (getAsyncWritePoolSize() > 0) { - asyncExecutor = (ThreadPoolExecutor) Executors.newFixedThreadPool(getAsyncWritePoolSize(), - new NamedThreadFactory("vfs-write-worker")); - } else { - asyncExecutor = new ImmediateExecutor(); - } - - return asyncExecutor; - } - - /** - * Returns ThreadPoolExecutor used to execute asynchronous write or touch jobs. - * @return ThreadPoolExecutor used to execute asynchronous write or touch jobs - */ - protected Executor getAsyncWriteExecutor() { - return asyncWriteExecutor; - } - /** * Returns the approximate number of threads that are actively executing asynchronous writing tasks. * @return the approximate number of threads that are actively executing asynchronous writing tasks @@ -777,8 +699,8 @@ private void deleteOlderRecursive(Set deleteIdSet, FileObject fo if (lastModified < timestamp) { identifier = new DataIdentifier(fileObject.getName().getBaseName()); - if (store.confirmDelete(identifier)) { - store.deleteFromCache(identifier); + if (getCachingDataStore().confirmDelete(identifier)) { + getCachingDataStore().deleteFromCache(identifier); if (LOG.isInfoEnabled()) { LOG.info("Deleting old file " + fileObject.getName().getFriendlyURI() + " modified: " @@ -797,17 +719,6 @@ private void deleteOlderRecursive(Set deleteIdSet, FileObject fo } } - /** - * This class implements {@link Executor} interface to run {@code command} right away, - * resulting in non-asynchronous mode executions. - */ - private class ImmediateExecutor implements Executor { - @Override - public void execute(Runnable command) { - command.run(); - } - } - /** * This class implements {@link Runnable} interface to copy {@link File} to VFS file object asynchronously. */ diff --git a/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSDataStore.java b/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSDataStore.java index 7cc778f1b4d..8acef86f63d 100644 --- a/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSDataStore.java +++ b/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSDataStore.java @@ -111,7 +111,7 @@ public class VFSDataStore extends CachingDataStore { /** * The pool size of asynchronous write pooling executor. */ - private int asyncWritePoolSize = VFSBackend.DEFAULT_ASYNC_WRITE_POOL_SIZE; + private int asyncWritePoolSize = 10; @Override public void init(String homeDir) throws RepositoryException { From 42f96c5d0ede9e102c49c513cc61160a7a701f90 Mon Sep 17 00:00:00 2001 From: Woonsan Ko Date: Fri, 26 Aug 2016 20:32:14 -0400 Subject: [PATCH 2/5] JCR-4006: double-checked locking on asyncWriteExecutor. By the way, the reason why lazy-initialization is because S3Backend and FSBackend have yet another init() method which makes it difficult to initialize once in the original init method. --- .../jackrabbit/core/data/AbstractBackend.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java index 4333c05630d..3507d2ded12 100644 --- a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java +++ b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java @@ -42,7 +42,7 @@ public abstract class AbstractBackend implements Backend { /** * Asynchronous write pooling executor. */ - private Executor asyncWriteExecutor; + private volatile Executor asyncWriteExecutor; /** * Returns the pool size of the asynchronous write pool executor. @@ -101,11 +101,18 @@ protected void setCachingDataStore(CachingDataStore cachingDataStore) { * @return Executor used to execute asynchronous write or touch jobs */ protected Executor getAsyncWriteExecutor() { - if (asyncWriteExecutor == null) { - asyncWriteExecutor = createAsyncWriteExecutor(); + Executor executor = asyncWriteExecutor; + + if (executor == null) { + synchronized (this) { + executor = asyncWriteExecutor; + if (executor == null) { + asyncWriteExecutor = executor = createAsyncWriteExecutor(); + } + } } - return asyncWriteExecutor; + return executor; } /** From bdb05c3c9e23cca6bd6083618724cbe392400674 Mon Sep 17 00:00:00 2001 From: Woonsan Ko Date: Fri, 26 Aug 2016 21:07:06 -0400 Subject: [PATCH 3/5] JCR-4006: enable TestCachingFDS#doDeleteRecordTest() with disabling async writing in unit test. --- .../java/org/apache/jackrabbit/core/data/FSBackend.java | 8 ++++++++ .../org/apache/jackrabbit/core/data/TestCachingFDS.java | 8 ++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java index 6b7b61a9b55..af63357fe3b 100644 --- a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java +++ b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java @@ -52,6 +52,8 @@ public class FSBackend extends AbstractBackend { public static final String FS_BACKEND_PATH = "fsBackendPath"; + public static final String ASYNC_WRITE_POOL_SIZE = "asyncWritePoolSize"; + /** * Logger instance. */ @@ -112,6 +114,12 @@ public void init(CachingDataStore store, String homeDir, Properties prop) + fsPathDir.getAbsolutePath()); } } + int asyncWritePoolSize = 10; + String value = prop.getProperty(ASYNC_WRITE_POOL_SIZE); + if (value != null) { + asyncWritePoolSize = Integer.parseInt(value); + } + setAsyncWritePoolSize(asyncWritePoolSize); } @Override diff --git a/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCachingFDS.java b/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCachingFDS.java index ae05cd5edb7..1f6b7f2d379 100644 --- a/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCachingFDS.java +++ b/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCachingFDS.java @@ -48,18 +48,14 @@ protected DataStore createDataStore() throws RepositoryException { } props.setProperty(FSBackend.FS_BACKEND_PATH, fsPath); LOG.info("fsBackendPath [{}] set.", fsPath); + // disable asynchronous writing in testing. + props.setProperty(FSBackend.ASYNC_WRITE_POOL_SIZE, "0"); cacheFDS.setProperties(props); cacheFDS.setSecret("12345"); cacheFDS.init(dataStoreDir); return cacheFDS; } - @Override - protected void doDeleteRecordTest() throws Exception { - ds = createDataStore(); - LOG.info("Skip deleteRecordTest on CachingFDS (JCR-4006)"); - } - /** * Test robustness of {@link AsyncUploadCache} corruption. */ From 00bdb6d4c3e06bfd0065263f1b5aeb65f51836c8 Mon Sep 17 00:00:00 2001 From: Woonsan Ko Date: Fri, 26 Aug 2016 21:18:16 -0400 Subject: [PATCH 4/5] JCR-4006: Add homeDir and config in AbstractBackend as clean up. --- .../jackrabbit/core/data/AbstractBackend.java | 44 +++++++++++++++++++ .../jackrabbit/core/data/FSBackend.java | 9 +--- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java index 3507d2ded12..cc0b1319116 100644 --- a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java +++ b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractBackend.java @@ -34,6 +34,16 @@ public abstract class AbstractBackend implements Backend { */ private CachingDataStore cachingDataStore; + /** + * path of repository home dir. + */ + private String homeDir; + + /** + * path of config property file. + */ + private String config; + /** * The pool size of asynchronous write pooling executor. */ @@ -66,6 +76,8 @@ public void setAsyncWritePoolSize(int asyncWritePoolSize) { @Override public void init(CachingDataStore cachingDataStore, String homeDir, String config) throws DataStoreException { this.cachingDataStore = cachingDataStore; + this.homeDir = homeDir; + this.config = config; } /** @@ -96,6 +108,38 @@ protected void setCachingDataStore(CachingDataStore cachingDataStore) { this.cachingDataStore = cachingDataStore; } + /** + * Returns path of repository home dir. + * @return path of repository home dir + */ + protected String getHomeDir() { + return homeDir; + } + + /** + * Sets path of repository home dir. + * @param homeDir path of repository home dir + */ + protected void setHomeDir(String homeDir) { + this.homeDir = homeDir; + } + + /** + * Returns path of config property file. + * @return path of config property file + */ + protected String getConfig() { + return config; + } + + /** + * Sets path of config property file. + * @param config path of config property file + */ + protected void setConfig(String config) { + this.config = config; + } + /** * Returns Executor used to execute asynchronous write or touch jobs. * @return Executor used to execute asynchronous write or touch jobs diff --git a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java index af63357fe3b..5187acc9373 100644 --- a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java +++ b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java @@ -44,10 +44,6 @@ public class FSBackend extends AbstractBackend { private String fsPath; - private String homeDir; - - private String config; - File fsPathDir; public static final String FS_BACKEND_PATH = "fsBackendPath"; @@ -72,7 +68,6 @@ public void init(CachingDataStore cachingDataStore, String homeDir, String confi Properties initProps = null; // Check is configuration is already provided. That takes precedence // over config provided via file based config - this.config = config; if (this.properties != null) { initProps = this.properties; } else { @@ -96,11 +91,11 @@ public void init(CachingDataStore cachingDataStore, String homeDir, String confi public void init(CachingDataStore store, String homeDir, Properties prop) throws DataStoreException { setCachingDataStore(store); - this.homeDir = homeDir; + setHomeDir(homeDir); this.fsPath = prop.getProperty(FS_BACKEND_PATH); if (this.fsPath == null || "".equals(this.fsPath)) { throw new DataStoreException("Could not initialize FSBackend from " - + config + ". [" + FS_BACKEND_PATH + "] property not found."); + + getConfig() + ". [" + FS_BACKEND_PATH + "] property not found."); } fsPathDir = new File(this.fsPath); if (fsPathDir.exists() && fsPathDir.isFile()) { From 30f5229805f2ed6a54dc2310c56324ad2c747bb9 Mon Sep 17 00:00:00 2001 From: Woonsan Ko Date: Fri, 26 Aug 2016 22:42:14 -0400 Subject: [PATCH 5/5] JCR-4006: disable async writing in TestCachingFDSCacheOff as well. --- .../org/apache/jackrabbit/core/data/TestCachingFDSCacheOff.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCachingFDSCacheOff.java b/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCachingFDSCacheOff.java index 09806766c4b..ac03e314ec7 100644 --- a/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCachingFDSCacheOff.java +++ b/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCachingFDSCacheOff.java @@ -40,6 +40,8 @@ protected DataStore createDataStore() throws RepositoryException { fsPath = dataStoreDir + "/cachingFDS"; } props.setProperty(FSBackend.FS_BACKEND_PATH, fsPath); + // disable asynchronous writing in testing. + props.setProperty(FSBackend.ASYNC_WRITE_POOL_SIZE, "0"); cacheFDS.setProperties(props); cacheFDS.setSecret("12345"); cacheFDS.setCacheSize(0);