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 2016/10/06 09:04:11 UTC
svn commit: r1763540 - in
/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak:
backup/impl/FileStoreBackupImpl.java segment/file/FileStore.java
segment/standby/client/StandbyClientSync.java segment/tool/Compact.java
Author: mduerig
Date: Thu Oct 6 09:04:11 2016
New Revision: 1763540
URL: http://svn.apache.org/viewvc?rev=1763540&view=rev
Log:
OAK-4895: FileStore cleanup should not leak out file handles
Make cleanup return void
Modified:
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/backup/impl/FileStoreBackupImpl.java
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/standby/client/StandbyClientSync.java
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/tool/Compact.java
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/backup/impl/FileStoreBackupImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/backup/impl/FileStoreBackupImpl.java?rev=1763540&r1=1763539&r2=1763540&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/backup/impl/FileStoreBackupImpl.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/backup/impl/FileStoreBackupImpl.java Thu Oct 6 09:04:11 2016
@@ -117,12 +117,7 @@ public class FileStoreBackupImpl impleme
@Override
public boolean cleanup(FileStore f) throws IOException {
- boolean ok = true;
-
- for (File file : f.cleanup()) {
- ok = ok && file.delete();
- }
-
+ f.cleanup();
return true;
}
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=1763540&r1=1763539&r2=1763540&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 Oct 6 09:04:11 2016
@@ -95,6 +95,7 @@ import org.apache.jackrabbit.oak.segment
import org.apache.jackrabbit.oak.segment.SegmentGraph.SegmentGraphVisitor;
import org.apache.jackrabbit.oak.segment.SegmentId;
import org.apache.jackrabbit.oak.segment.SegmentIdFactory;
+import org.apache.jackrabbit.oak.segment.SegmentIdTable;
import org.apache.jackrabbit.oak.segment.SegmentNodeState;
import org.apache.jackrabbit.oak.segment.SegmentNotFoundException;
import org.apache.jackrabbit.oak.segment.SegmentReader;
@@ -765,8 +766,18 @@ public class FileStore implements Segmen
* Those tar files that shrink by at least 25% are rewritten to a new tar generation
* skipping the reclaimed segments.
*/
- public List<File> cleanup() throws IOException {
- int gcGeneration = getGcGeneration();
+ public void cleanup() throws IOException {
+ fileReaper.add(cleanupOldGenerations(getGcGeneration()));
+ }
+
+ /**
+ * Cleanup segments that are from an old generation. That segments whose generation
+ * is {@code gcGeneration - SegmentGCOptions.getRetainedGenerations()} or older.
+ * @param gcGeneration
+ * @return list of files to be removed
+ * @throws IOException
+ */
+ private List<File> cleanupOldGenerations(int gcGeneration) throws IOException {
final int reclaimGeneration = gcGeneration - gcOptions.getRetainedGenerations();
Predicate<Integer> reclaimPredicate = new Predicate<Integer>() {
@@ -782,6 +793,33 @@ public class FileStore implements Segmen
",reclaim-predicate=(generation<=" + reclaimGeneration + ")");
}
+ /**
+ * Cleanup segments of the given generation {@code gcGeneration}.
+ * @param gcGeneration
+ * @return list of files to be removed
+ * @throws IOException
+ */
+ private List<File> cleanupGeneration(final int gcGeneration) throws IOException {
+ Predicate<Integer> cleanupPredicate = new Predicate<Integer>() {
+ @Override
+ public boolean apply(Integer generation) {
+ return generation == gcGeneration;
+ }
+ };
+ return cleanup(cleanupPredicate,
+ "gc-count=" + GC_COUNT +
+ ",gc-status=failed" +
+ ",store-generation=" + (gcGeneration - 1) +
+ ",reclaim-predicate=(generation==" + gcGeneration + ")");
+ }
+
+ /**
+ * Cleanup segments whose generation matches the {@code reclaimGeneration} predicate.
+ * @param reclaimGeneration
+ * @param gcInfo gc information to be passed to {@link SegmentIdTable#clearSegmentIdTables(Set, String)}
+ * @return list of files to be removed
+ * @throws IOException
+ */
private List<File> cleanup(
@Nonnull Predicate<Integer> reclaimGeneration,
@Nonnull String gcInfo)
@@ -1048,7 +1086,7 @@ public class FileStore implements Segmen
@Override
public void run() {
try {
- fileReaper.add(cleanup());
+ fileReaper.add(cleanupOldGenerations(newGeneration));
} catch (IOException e) {
gcListener.error("TarMK GC #" + GC_COUNT + ": cleanup failed", e);
}
@@ -1063,17 +1101,7 @@ public class FileStore implements Segmen
public void run() {
try {
gcListener.info("TarMK GC #{}: cleaning up after failed compaction", GC_COUNT);
- Predicate<Integer> cleanupPredicate = new Predicate<Integer>() {
- @Override
- public boolean apply(Integer generation) {
- return generation == newGeneration;
- }
- };
- fileReaper.add(cleanup(cleanupPredicate,
- "gc-count=" + GC_COUNT +
- ",gc-status=failed" +
- ",store-generation=" + (newGeneration - 1) +
- ",reclaim-predicate=(generation==" + newGeneration + ")"));
+ fileReaper.add(cleanupGeneration(newGeneration));
} catch (IOException e) {
gcListener.error("TarMK GC #" + GC_COUNT + ": cleanup failed", e);
}
@@ -1226,6 +1254,8 @@ public class FileStore implements Segmen
"Failed to close the TarMK at " + directory, e);
}
+ // Try removing pending files in case the scheduler didn't have a chance to run yet
+ fileReaper.reap();
System.gc(); // for any memory-mappings that are no longer used
log.info("TarMK closed: {}", directory);
@@ -1579,7 +1609,7 @@ public class FileStore implements Segmen
public void flush() { /* nop */ }
@Override
- public LinkedList<File> cleanup() {
+ public void cleanup() {
throw new UnsupportedOperationException("Read Only Store");
}
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/standby/client/StandbyClientSync.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/standby/client/StandbyClientSync.java?rev=1763540&r1=1763539&r2=1763540&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/standby/client/StandbyClientSync.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/standby/client/StandbyClientSync.java Thu Oct 6 09:04:11 2016
@@ -22,10 +22,8 @@ package org.apache.jackrabbit.oak.segmen
import static org.apache.jackrabbit.oak.commons.IOUtils.humanReadableByteCount;
import java.io.Closeable;
-import java.io.File;
import java.io.IOException;
import java.lang.management.ManagementFactory;
-import java.nio.file.Files;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -165,15 +163,7 @@ public final class StandbyClientSync imp
}
private void cleanupAndRemove() throws IOException {
- for (File file : fileStore.cleanup()) {
- log.info("Removing file {}", file);
-
- try {
- Files.deleteIfExists(file.toPath());
- } catch (IOException e) {
- log.warn(String.format("Unable to remove file %s", file), e);
- }
- }
+ fileStore.cleanup();
}
private Supplier<Boolean> newRunningSupplier() {
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/tool/Compact.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/tool/Compact.java?rev=1763540&r1=1763539&r2=1763540&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/tool/Compact.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/tool/Compact.java Thu Oct 6 09:04:11 2016
@@ -119,18 +119,9 @@ public class Compact implements Runnable
System.out.println(" -> cleaning up");
try (FileStore store = newFileStore()) {
- for (File file : store.cleanup()) {
- if (!file.exists() || file.delete()) {
- System.out.println(" -> removed old file " + file.getName());
- } else {
- System.out.println(" -> failed to remove old file " + file.getName());
- }
- }
-
- String head;
-
+ store.cleanup();
File journal = new File(path, "journal.log");
-
+ String head;
try (JournalReader journalReader = new JournalReader(journal)) {
head = journalReader.next() + " root " + System.currentTimeMillis() + "\n";
}