You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by md...@apache.org on 2017/12/07 11:27:30 UTC
svn commit: r1817357 - in
/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file:
FileStore.java ShutDown.java
Author: mduerig
Date: Thu Dec 7 11:27:30 2017
New Revision: 1817357
URL: http://svn.apache.org/viewvc?rev=1817357&view=rev
Log:
OAK-6884: TarMK disk space check is not synchronized with FileStore opened state
Avoid scheduling further disk space checks once a shutdown has been initiated
Modified:
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/ShutDown.java
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java?rev=1817357&r1=1817356&r2=1817357&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java Thu Dec 7 11:27:30 2017
@@ -205,39 +205,37 @@ public class FileStore extends AbstractF
this.snfeListener = builder.getSnfeListener();
- fileStoreScheduler.scheduleAtFixedRate(
- format("TarMK flush [%s]", directory), 5, SECONDS,
- new Runnable() {
- @Override
- public void run() {
- if (shutDown.shutDownRequested()) {
- return;
- }
- try {
- tryFlush();
- } catch (IOException e) {
- log.warn("Failed to flush the TarMK at {}", directory, e);
- }
- }
- });
- fileStoreScheduler.scheduleAtFixedRate(
- format("TarMK filer reaper [%s]", directory), 5, SECONDS,
- new Runnable() {
- @Override
- public void run() {
- fileReaper.reap();
- }
- });
- fileStoreScheduler.scheduleAtFixedRate(
- format("TarMK disk space check [%s]", directory), 1, MINUTES,
- new Runnable() {
- final SegmentGCOptions gcOptions = builder.getGcOptions();
-
- @Override
- public void run() {
- checkDiskSpace(gcOptions);
- }
- });
+ fileStoreScheduler.scheduleAtFixedRate(format("TarMK flush [%s]", directory), 5, SECONDS, () -> {
+ try (ShutDownCloser ignore = shutDown.tryKeepAlive()) {
+ if (shutDown.isShutDown()) {
+ log.debug("Shut down in progress, skipping flush");
+ } else if (revisions == null) {
+ log.debug("No TarRevisions available, skipping flush");
+ } else {
+ revisions.tryFlush(() -> {
+ segmentWriter.flush();
+ tarFiles.flush();
+ stats.flushed();
+ });
+ }
+ } catch (IOException e) {
+ log.warn("Failed to flush the TarMK at {}", directory, e);
+ }
+ });
+
+ fileStoreScheduler.scheduleAtFixedRate(format("TarMK filer reaper [%s]", directory), 5, SECONDS,
+ fileReaper::reap);
+
+ fileStoreScheduler.scheduleAtFixedRate(format("TarMK disk space check [%s]", directory), 1, MINUTES, () -> {
+ try (ShutDownCloser ignore = shutDown.tryKeepAlive()) {
+ if (shutDown.isShutDown()) {
+ log.debug("Shut down in progress, skipping disk space check");
+ } else {
+ checkDiskSpace(builder.getGcOptions());
+ }
+ }
+ });
+
log.info("TarMK opened at {}, mmap={}, size={} ({} bytes)",
directory,
memoryMapping,
@@ -320,24 +318,9 @@ public class FileStore extends AbstractF
return stats;
}
- private void doTryFlush() throws IOException {
- if (revisions == null) {
- log.debug("No TarRevisions available, skipping flush");
- return;
- }
- revisions.tryFlush(() -> {
- segmentWriter.flush();
- tarFiles.flush();
- stats.flushed();
- });
- }
-
- private void tryFlush() throws IOException {
- try (ShutDownCloser ignored = shutDown.keepAlive()) {
- doTryFlush();
- }
- }
-
+ /*
+ * Callers of this method must hold the shutdown lock
+ */
private void doFlush() throws IOException {
if (revisions == null) {
log.debug("No TarRevisions available, skipping flush");
@@ -1143,7 +1126,7 @@ public class FileStore extends AbstractF
reason = "Not enough memory";
return true;
}
- if (store.shutDown.shutDownRequested()) {
+ if (store.shutDown.isShutDown()) {
reason = "The FileStore is shutting down";
return true;
}
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/ShutDown.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/ShutDown.java?rev=1817357&r1=1817356&r2=1817357&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/ShutDown.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/ShutDown.java Thu Dec 7 11:27:30 2017
@@ -20,20 +20,42 @@ package org.apache.jackrabbit.oak.segmen
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
+/**
+ * An instance of this class encapsulates the shutdown logic of the {@link FileStore}.
+ * <p>
+ * A shutdown is initiated by calling {@link #shutDown} and completed at the point
+ * where the returned {@link ShutDownCloser} has been {@link ShutDownCloser#close() closed}.
+ *
+ * Code that needs to protect itself from running after a shutdown has been initiated can
+ * use the {@link #keepAlive()}, {@link #tryKeepAlive()} and {@link #isShutDown}:
+ *
+ * <pre>
+ try (ShutDownCloser ignored = shutDown.keepAlive()) {
+ // protected code here
+ }
+ * </pre>
+ */
class ShutDown {
+ /**
+ * An {@link AutoCloseable} whose {@link #close()} doesn't throw an exception.
+ */
interface ShutDownCloser extends AutoCloseable {
-
+ @Override
void close();
-
}
- private volatile boolean shutDownRequested;
+ private volatile boolean isShutDown;
private boolean shutDown;
private final ReadWriteLock lock = new ReentrantReadWriteLock();
+ /**
+ * Keep the store from being shut down until the returned {@link ShutDownCloser}
+ * is {@link ShutDownCloser#close() closed}.
+ * @throws IllegalStateException if the store is already {@link #isShutDown() shut down}.
+ */
ShutDownCloser keepAlive() {
lock.readLock().lock();
@@ -45,8 +67,23 @@ class ShutDown {
return () -> lock.readLock().unlock();
}
+ /**
+ * Try to keep the store from being shut down until the returned {@link ShutDownCloser}
+ * is {@link ShutDownCloser#close() closed}. Callers of this method need to call
+ * {@link #isShutDown()} before proceeding.
+ */
+ ShutDownCloser tryKeepAlive() {
+ lock.readLock().lock();
+ return () -> lock.readLock().unlock();
+ }
+
+ /**
+ * Initiate a shutdown of the store. The shutdown is complete once the returned
+ * {@link ShutDownCloser} is {@link ShutDownCloser#close() closed}.
+ * @return
+ */
ShutDownCloser shutDown() {
- shutDownRequested = true;
+ isShutDown = true;
lock.writeLock().lock();
if (shutDown) {
@@ -60,8 +97,11 @@ class ShutDown {
};
}
- boolean shutDownRequested() {
- return shutDownRequested;
+ /**
+ * @return {@code true} iff the store is shut down.
+ */
+ boolean isShutDown() {
+ return isShutDown;
}
}