Thanks Amit for confirming it! Chetan Mehrotra
On Fri, Mar 13, 2015 at 11:53 AM, Amit Jain <[email protected]> wrote: > The tests are passing for me on windows with the latest change. > > Thanks > Amit > > On Fri, Mar 13, 2015 at 9:21 AM, Chetan Mehrotra <[email protected]> > wrote: > >> Looks like Closer closes the closeables in LIFO manner due to which >> directory containing that file got deleted first. I have change the >> logic now. >> >> Let me know if the test passes for you on Windows >> Chetan Mehrotra >> >> >> On Thu, Mar 12, 2015 at 10:21 PM, Julian Reschke <[email protected]> >> wrote: >> > With this change, I get a reliable test failure on Windows: >> > >> > >> > Tests in error: >> > >> > overflowToDisk(org.apache.jackrabbit.oak.commons.sort.StringSortTest): >> > Unable to delete file: >> C:\tmp\oak-sorter-1426178913437-0\strings-sorted.txt >> > >> > >> > Best regards, Julian >> > >> > >> > On 2015-03-12 16:22, [email protected] wrote: >> >> >> >> Author: chetanm >> >> Date: Thu Mar 12 15:22:46 2015 >> >> New Revision: 1666220 >> >> >> >> URL: http://svn.apache.org/r1666220 >> >> Log: >> >> OAK-2557 - VersionGC uses way too much memory if there is a large pile >> of >> >> garbage >> >> >> >> Added: >> >> >> >> >> jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/StringSort.java >> >> (with props) >> >> >> >> >> jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/sort/StringSortTest.java >> >> (with props) >> >> Modified: >> >> jackrabbit/oak/trunk/oak-commons/pom.xml >> >> >> >> >> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java >> >> >> >> >> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java >> >> >> >> >> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCDeletionTest.java >> >> >> >> >> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCWithSplitTest.java >> >> >> >> Modified: jackrabbit/oak/trunk/oak-commons/pom.xml >> >> URL: >> >> >> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-commons/pom.xml?rev=1666220&r1=1666219&r2=1666220&view=diff >> >> >> >> >> ============================================================================== >> >> --- jackrabbit/oak/trunk/oak-commons/pom.xml (original) >> >> +++ jackrabbit/oak/trunk/oak-commons/pom.xml Thu Mar 12 15:22:46 2015 >> >> @@ -93,6 +93,11 @@ >> >> <artifactId>oak-mk-api</artifactId> >> >> <version>${project.version}</version> >> >> </dependency> >> >> + <dependency> >> >> + <groupId>commons-io</groupId> >> >> + <artifactId>commons-io</artifactId> >> >> + <version>2.4</version> >> >> + </dependency> >> >> >> >> <!-- Test dependencies --> >> >> <dependency> >> >> >> >> Added: >> >> >> jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/StringSort.java >> >> URL: >> >> >> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/StringSort.java?rev=1666220&view=auto >> >> >> >> >> ============================================================================== >> >> --- >> >> >> jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/StringSort.java >> >> (added) >> >> +++ >> >> >> jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/StringSort.java >> >> Thu Mar 12 15:22:46 2015 >> >> @@ -0,0 +1,255 @@ >> >> +/* >> >> + * 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.oak.commons.sort; >> >> + >> >> +import java.io.BufferedWriter; >> >> +import java.io.Closeable; >> >> +import java.io.File; >> >> +import java.io.FileNotFoundException; >> >> +import java.io.IOException; >> >> +import java.io.Reader; >> >> +import java.nio.charset.Charset; >> >> +import java.util.Collections; >> >> +import java.util.Comparator; >> >> +import java.util.Iterator; >> >> +import java.util.List; >> >> + >> >> +import com.google.common.base.Charsets; >> >> +import com.google.common.collect.Lists; >> >> +import com.google.common.io.Closer; >> >> +import com.google.common.io.Files; >> >> +import org.apache.commons.io.FileUtils; >> >> +import org.apache.commons.io.LineIterator; >> >> +import org.slf4j.Logger; >> >> +import org.slf4j.LoggerFactory; >> >> + >> >> +/** >> >> + * Utility class to store a list of string and perform sort on that. >> For >> >> small size >> >> + * the list would be maintained in memory. If the size crosses the >> >> required threshold then >> >> + * the sorting would be performed externally >> >> + */ >> >> +public class StringSort implements Closeable { >> >> + private final Logger log = LoggerFactory.getLogger(getClass()); >> >> + public static final int BATCH_SIZE = 2048; >> >> + >> >> + private final int overflowToDiskThreshold; >> >> + private final Comparator<String> comparator; >> >> + >> >> + private final List<String> ids = Lists.newArrayList(); >> >> + private long size; >> >> + >> >> + private final List<String> inMemBatch = Lists.newArrayList(); >> >> + >> >> + private boolean useFile; >> >> + private PersistentState persistentState; >> >> + >> >> + public StringSort(int overflowToDiskThreshold, Comparator<String> >> >> comparator) { >> >> + this.overflowToDiskThreshold = overflowToDiskThreshold; >> >> + this.comparator = comparator; >> >> + } >> >> + >> >> + public void add(String id) throws IOException { >> >> + if (useFile) { >> >> + addToBatch(id); >> >> + } else { >> >> + ids.add(id); >> >> + if (ids.size() >= overflowToDiskThreshold) { >> >> + flushToFile(ids); >> >> + useFile = true; >> >> + log.debug("In memory buffer crossed the threshold of >> {}. >> >> " + >> >> + "Switching to filesystem [{}] to manage the >> >> state", overflowToDiskThreshold, persistentState); >> >> + } >> >> + } >> >> + size++; >> >> + } >> >> + >> >> + public void sort() throws IOException { >> >> + if (useFile) { >> >> + //Flush the last batch >> >> + flushToFile(inMemBatch); >> >> + persistentState.sort(); >> >> + } else { >> >> + Collections.sort(ids, comparator); >> >> + } >> >> + } >> >> + >> >> + public Iterator<String> getIds() throws IOException { >> >> + if (useFile) { >> >> + return persistentState.getIterator(); >> >> + } else { >> >> + return ids.iterator(); >> >> + } >> >> + } >> >> + >> >> + public long getSize() { >> >> + return size; >> >> + } >> >> + >> >> + public boolean isEmpty() { >> >> + return size == 0; >> >> + } >> >> + >> >> + public boolean usingFile() { >> >> + return useFile; >> >> + } >> >> + >> >> + @Override >> >> + public void close() throws IOException { >> >> + if (persistentState != null) { >> >> + persistentState.close(); >> >> + } >> >> + } >> >> + >> >> + private void addToBatch(String id) throws IOException { >> >> + inMemBatch.add(id); >> >> + if (inMemBatch.size() >= BATCH_SIZE) { >> >> + flushToFile(inMemBatch); >> >> + } >> >> + } >> >> + >> >> + private void flushToFile(List<String> ids) throws IOException { >> >> + BufferedWriter w = getPersistentState().getWriter(); >> >> + for (String id : ids) { >> >> + w.write(id); >> >> + w.newLine(); >> >> + } >> >> + ids.clear(); >> >> + } >> >> + >> >> + private PersistentState getPersistentState() { >> >> + //Lazily initialize the persistent state >> >> + if (persistentState == null) { >> >> + persistentState = new PersistentState(comparator); >> >> + } >> >> + return persistentState; >> >> + } >> >> + >> >> + private static class PersistentState implements Closeable { >> >> + /** >> >> + * Maximum loop count when creating temp directories. >> >> + */ >> >> + private static final int TEMP_DIR_ATTEMPTS = 10000; >> >> + >> >> + private final Charset charset = Charsets.UTF_8; >> >> + private final File workDir; >> >> + private final Comparator<String> comparator; >> >> + private File idFile; >> >> + private File sortedFile; >> >> + private BufferedWriter writer; >> >> + private List<CloseableIterator> openedIterators = >> >> Lists.newArrayList(); >> >> + >> >> + public PersistentState(Comparator<String> comparator) { >> >> + this(comparator, createTempDir("oak-sorter-")); >> >> + } >> >> + >> >> + public PersistentState(Comparator<String> comparator, File >> >> workDir) { >> >> + this.workDir = workDir; >> >> + this.comparator = comparator; >> >> + } >> >> + >> >> + public BufferedWriter getWriter() throws FileNotFoundException >> { >> >> + if (idFile == null) { >> >> + idFile = new File(workDir, "strings.txt"); >> >> + sortedFile = new File(workDir, "strings-sorted.txt"); >> >> + writer = Files.newWriter(idFile, charset); >> >> + } >> >> + return writer; >> >> + } >> >> + >> >> + public void sort() throws IOException { >> >> + closeWriter(); >> >> + >> >> + List<File> sortedFiles = ExternalSort.sortInBatch(idFile, >> >> + comparator, //Comparator to use >> >> + ExternalSort.DEFAULTMAXTEMPFILES, >> >> + ExternalSort.DEFAULT_MAX_MEM_BYTES, >> >> + charset, //charset >> >> + workDir, //temp directory where intermediate files >> >> are created >> >> + true //distinct >> >> + ); >> >> + >> >> + ExternalSort.mergeSortedFiles(sortedFiles, >> >> + sortedFile, >> >> + comparator, >> >> + charset, >> >> + true >> >> + ); >> >> + } >> >> + >> >> + public Iterator<String> getIterator() throws IOException { >> >> + CloseableIterator itr = new >> >> CloseableIterator(Files.newReader(sortedFile, charset)); >> >> + openedIterators.add(itr); >> >> + return itr; >> >> + } >> >> + >> >> + @Override >> >> + public String toString() { >> >> + return "PersistentState : workDir=" + >> >> workDir.getAbsolutePath(); >> >> + } >> >> + >> >> + @Override >> >> + public void close() throws IOException { >> >> + Closer closer = Closer.create(); >> >> + try { >> >> + closer.register(writer); >> >> + for (CloseableIterator citr : openedIterators) { >> >> + closer.register(citr); >> >> + } >> >> + closer.register(new Closeable() { >> >> + @Override >> >> + public void close() throws IOException { >> >> + FileUtils.deleteDirectory(workDir); >> >> + } >> >> + }); >> >> + } finally { >> >> + closer.close(); >> >> + } >> >> + } >> >> + >> >> + private void closeWriter() throws IOException { >> >> + writer.close(); >> >> + } >> >> + >> >> + /** >> >> + * Taken from com.google.common.io.Files#createTempDir() >> >> + * Modified to provide a prefix >> >> + */ >> >> + private static File createTempDir(String prefix) { >> >> + File baseDir = new >> >> File(System.getProperty("java.io.tmpdir")); >> >> + String baseName = System.currentTimeMillis() + "-"; >> >> + >> >> + for (int counter = 0; counter < TEMP_DIR_ATTEMPTS; >> counter++) >> >> { >> >> + File tempDir = new File(baseDir, prefix + baseName + >> >> counter); >> >> + if (tempDir.mkdir()) { >> >> + return tempDir; >> >> + } >> >> + } >> >> + throw new IllegalStateException("Failed to create directory >> >> within " >> >> + + TEMP_DIR_ATTEMPTS + " attempts (tried " >> >> + + baseName + "0 to " + baseName + >> (TEMP_DIR_ATTEMPTS >> >> - 1) + ')'); >> >> + } >> >> + } >> >> + >> >> + private static class CloseableIterator extends LineIterator >> >> implements Closeable { >> >> + public CloseableIterator(Reader reader) throws >> >> IllegalArgumentException { >> >> + super(reader); >> >> + } >> >> + } >> >> +} >> >> >> >> Propchange: >> >> >> jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/StringSort.java >> >> >> >> >> ------------------------------------------------------------------------------ >> >> svn:eol-style = native >> >> >> >> Added: >> >> >> jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/sort/StringSortTest.java >> >> URL: >> >> >> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/sort/StringSortTest.java?rev=1666220&view=auto >> >> >> >> >> ============================================================================== >> >> --- >> >> >> jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/sort/StringSortTest.java >> >> (added) >> >> +++ >> >> >> jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/sort/StringSortTest.java >> >> Thu Mar 12 15:22:46 2015 >> >> @@ -0,0 +1,144 @@ >> >> +/* >> >> + * 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.oak.commons.sort; >> >> + >> >> +import java.io.IOException; >> >> +import java.io.Serializable; >> >> +import java.util.ArrayList; >> >> +import java.util.Arrays; >> >> +import java.util.Collections; >> >> +import java.util.Comparator; >> >> +import java.util.HashSet; >> >> +import java.util.List; >> >> +import java.util.Set; >> >> + >> >> +import com.google.common.base.Joiner; >> >> +import com.google.common.collect.Collections2; >> >> +import com.google.common.collect.ImmutableList; >> >> +import org.junit.Test; >> >> + >> >> +import static org.junit.Assert.assertEquals; >> >> +import static org.junit.Assert.assertFalse; >> >> +import static org.junit.Assert.assertTrue; >> >> + >> >> +public class StringSortTest { >> >> + private Comparator<String> comparator = new PathComparator(); >> >> + private StringSort collector; >> >> + >> >> + @Test >> >> + public void inMemory() throws Exception{ >> >> + List<String> paths = createTestPaths(5, false); >> >> + collector = new StringSort(paths.size() + 1,comparator); >> >> + addPathsToCollector(paths); >> >> + >> >> + assertConstraints(paths); >> >> + assertFalse(collector.usingFile()); >> >> + collector.close(); >> >> + } >> >> + >> >> + @Test >> >> + public void overflowToDisk() throws Exception{ >> >> + //Create ~50k paths >> >> + List<String> paths = createTestPaths(10, true); >> >> + collector = new StringSort(1000, comparator); >> >> + addPathsToCollector(paths); >> >> + >> >> + assertTrue(collector.usingFile()); >> >> + assertConstraints(paths); >> >> + >> >> + collector.close(); >> >> + } >> >> + >> >> + private void assertConstraints(List<String> paths) throws >> IOException >> >> { >> >> + assertEquals(paths.size(), collector.getSize()); >> >> + >> >> + Collections.sort(paths, comparator); >> >> + collector.sort(); >> >> + >> >> + List<String> sortedPaths = >> >> ImmutableList.copyOf(collector.getIds()); >> >> + assertEquals(paths.size(), sortedPaths.size()); >> >> + assertEquals(paths, sortedPaths); >> >> + } >> >> + >> >> + private void addPathsToCollector(Iterable<String> paths) throws >> >> IOException { >> >> + for (String path : paths){ >> >> + collector.add(path); >> >> + } >> >> + } >> >> + >> >> + private static List<String> createTestPaths(int depth, boolean >> >> permutation){ >> >> + List<String> rootPaths = Arrays.asList("a", "b", "c", "d", "e", >> >> "f", "g"); >> >> + List<String> paths = new ArrayList<String>(); >> >> + >> >> + >> >> + if (permutation){ >> >> + List<String> newRoots = new ArrayList<String>(); >> >> + for (List<String> permuts : >> >> Collections2.orderedPermutations(rootPaths)){ >> >> + newRoots.add(Joiner.on("").join(permuts)); >> >> + } >> >> + rootPaths = newRoots; >> >> + } >> >> + >> >> + for (String root : rootPaths){ >> >> + List<String> pathElements = new ArrayList<String>(); >> >> + pathElements.add(root); >> >> + paths.add(createId(pathElements)); >> >> + for (int i = 0; i < depth; i++){ >> >> + pathElements.add(root + i); >> >> + paths.add(createId(pathElements)); >> >> + } >> >> + } >> >> + >> >> + Set<String> idSet = new HashSet<String>(paths); >> >> + assertEquals(paths.size(), idSet.size()); >> >> + >> >> + Collections.shuffle(paths); >> >> + return paths; >> >> + } >> >> + >> >> + private static String createId(Iterable<String> pathElements){ >> >> + return "/" + Joiner.on('/').join(pathElements); >> >> + } >> >> + >> >> + private static class PathComparator implements Comparator<String>, >> >> Serializable { >> >> + @Override >> >> + public int compare(String o1, String o2) { >> >> + int d1 = pathDepth(o1); >> >> + int d2 = pathDepth(o2); >> >> + if (d1 != d2) { >> >> + return Integer.signum(d2 - d1); >> >> + } >> >> + return o1.compareTo(o2); >> >> + } >> >> + >> >> + private static int pathDepth(String path) { >> >> + if (path.equals("/")) { >> >> + return 0; >> >> + } >> >> + int depth = 0; >> >> + for (int i = 0; i < path.length(); i++) { >> >> + if (path.charAt(i) == '/') { >> >> + depth++; >> >> + } >> >> + } >> >> + return depth; >> >> + } >> >> + } >> >> +} >> >> >> >> Propchange: >> >> >> jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/sort/StringSortTest.java >> >> >> >> >> ------------------------------------------------------------------------------ >> >> svn:eol-style = native >> >> >> >> Modified: >> >> >> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java >> >> URL: >> >> >> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java?rev=1666220&r1=1666219&r2=1666220&view=diff >> >> >> >> >> ============================================================================== >> >> --- >> >> >> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java >> >> (original) >> >> +++ >> >> >> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java >> >> Thu Mar 12 15:22:46 2015 >> >> @@ -574,7 +574,11 @@ public class DocumentNodeStoreService { >> >> RevisionGC revisionGC = new RevisionGC(new Runnable() { >> >> @Override >> >> public void run() { >> >> - >> >> store.getVersionGarbageCollector().gc(versionGcMaxAgeInSecs, >> >> TimeUnit.SECONDS); >> >> + try { >> >> + >> >> store.getVersionGarbageCollector().gc(versionGcMaxAgeInSecs, >> >> TimeUnit.SECONDS); >> >> + } catch (IOException e) { >> >> + log.warn("Error occurred while executing the >> Version >> >> Garbage Collector", e); >> >> + } >> >> } >> >> }, executor); >> >> registrations.add(registerMBean(whiteboard, >> >> RevisionGCMBean.class, revisionGC, >> >> >> >> Modified: >> >> >> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java >> >> URL: >> >> >> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java?rev=1666220&r1=1666219&r2=1666220&view=diff >> >> >> >> >> ============================================================================== >> >> --- >> >> >> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java >> >> (original) >> >> +++ >> >> >> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java >> >> Thu Mar 12 15:22:46 2015 >> >> @@ -19,9 +19,9 @@ >> >> >> >> package org.apache.jackrabbit.oak.plugins.document; >> >> >> >> -import java.util.ArrayList; >> >> -import java.util.Collections; >> >> +import java.io.IOException; >> >> import java.util.EnumSet; >> >> +import java.util.Iterator; >> >> import java.util.List; >> >> import java.util.Set; >> >> import java.util.concurrent.TimeUnit; >> >> @@ -31,18 +31,23 @@ import com.google.common.base.StandardSy >> >> import com.google.common.base.Stopwatch; >> >> import com.google.common.collect.ImmutableList; >> >> >> >> +import org.apache.jackrabbit.oak.commons.sort.StringSort; >> >> import org.apache.jackrabbit.oak.plugins.document.util.Utils; >> >> import org.slf4j.Logger; >> >> import org.slf4j.LoggerFactory; >> >> >> >> +import static com.google.common.collect.Iterators.partition; >> >> import static >> >> >> org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType.COMMIT_ROOT_ONLY; >> >> import static >> >> >> org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType.DEFAULT_LEAF; >> >> >> >> public class VersionGarbageCollector { >> >> + //Kept less than MongoDocumentStore.IN_CLAUSE_BATCH_SIZE to avoid >> >> re-partitioning >> >> + private static final int DELETE_BATCH_SIZE = 450; >> >> private final DocumentNodeStore nodeStore; >> >> private final VersionGCSupport versionStore; >> >> + private int overflowToDiskThreshold = 100000; >> >> >> >> - private final Logger log = LoggerFactory.getLogger(getClass()); >> >> + private static final Logger log = >> >> LoggerFactory.getLogger(VersionGarbageCollector.class); >> >> >> >> /** >> >> * Split document types which can be safely garbage collected >> >> @@ -56,7 +61,7 @@ public class VersionGarbageCollector { >> >> this.versionStore = gcSupport; >> >> } >> >> >> >> - public VersionGCStats gc(long maxRevisionAge, TimeUnit unit) { >> >> + public VersionGCStats gc(long maxRevisionAge, TimeUnit unit) throws >> >> IOException { >> >> long maxRevisionAgeInMillis = unit.toMillis(maxRevisionAge); >> >> Stopwatch sw = Stopwatch.createStarted(); >> >> VersionGCStats stats = new VersionGCStats(); >> >> @@ -85,41 +90,60 @@ public class VersionGarbageCollector { >> >> return stats; >> >> } >> >> >> >> + public void setOverflowToDiskThreshold(int >> overflowToDiskThreshold) { >> >> + this.overflowToDiskThreshold = overflowToDiskThreshold; >> >> + } >> >> + >> >> private void collectSplitDocuments(VersionGCStats stats, long >> >> oldestRevTimeStamp) { >> >> versionStore.deleteSplitDocuments(GC_TYPES, >> oldestRevTimeStamp, >> >> stats); >> >> } >> >> >> >> - private void collectDeletedDocuments(VersionGCStats stats, Revision >> >> headRevision, long oldestRevTimeStamp) { >> >> - List<String> docIdsToDelete = new ArrayList<String>(); >> >> - Iterable<NodeDocument> itr = >> >> versionStore.getPossiblyDeletedDocs(oldestRevTimeStamp); >> >> + private void collectDeletedDocuments(VersionGCStats stats, Revision >> >> headRevision, long oldestRevTimeStamp) >> >> + throws IOException { >> >> + StringSort docIdsToDelete = new >> >> StringSort(overflowToDiskThreshold, NodeDocumentIdComparator.INSTANCE); >> >> try { >> >> - for (NodeDocument doc : itr) { >> >> - //Check if node is actually deleted at current revision >> >> - //As node is not modified since oldestRevTimeStamp then >> >> - //this node has not be revived again in past >> >> maxRevisionAge >> >> - //So deleting it is safe >> >> - if (doc.getNodeAtRevision(nodeStore, headRevision, >> null) >> >> == null) { >> >> - docIdsToDelete.add(doc.getId()); >> >> - //Collect id of all previous docs also >> >> - for (NodeDocument prevDoc : >> >> ImmutableList.copyOf(doc.getAllPreviousDocs())) { >> >> - docIdsToDelete.add(prevDoc.getId()); >> >> + Iterable<NodeDocument> itr = >> >> versionStore.getPossiblyDeletedDocs(oldestRevTimeStamp); >> >> + try { >> >> + for (NodeDocument doc : itr) { >> >> + //Check if node is actually deleted at current >> >> revision >> >> + //As node is not modified since oldestRevTimeStamp >> >> then >> >> + //this node has not be revived again in past >> >> maxRevisionAge >> >> + //So deleting it is safe >> >> + if (doc.getNodeAtRevision(nodeStore, headRevision, >> >> null) == null) { >> >> + docIdsToDelete.add(doc.getId()); >> >> + //Collect id of all previous docs also >> >> + for (NodeDocument prevDoc : >> >> ImmutableList.copyOf(doc.getAllPreviousDocs())) { >> >> + docIdsToDelete.add(prevDoc.getId()); >> >> + } >> >> } >> >> } >> >> + } finally { >> >> + Utils.closeIfCloseable(itr); >> >> + } >> >> + >> >> + if (docIdsToDelete.isEmpty()){ >> >> + return; >> >> } >> >> - } finally { >> >> - Utils.closeIfCloseable(itr); >> >> - } >> >> >> >> - Collections.sort(docIdsToDelete, >> >> NodeDocumentIdComparator.INSTANCE); >> >> + docIdsToDelete.sort(); >> >> + log.info("Proceeding to delete [{}] documents", >> >> docIdsToDelete.getSize()); >> >> >> >> - if(log.isDebugEnabled()) { >> >> - StringBuilder sb = new StringBuilder("Deleted document with >> >> following ids were deleted as part of GC \n"); >> >> - >> >> Joiner.on(StandardSystemProperty.LINE_SEPARATOR.value()).appendTo(sb, >> >> docIdsToDelete); >> >> - log.debug(sb.toString()); >> >> + if (log.isDebugEnabled() && docIdsToDelete.getSize() < >> 1000) >> >> { >> >> + StringBuilder sb = new StringBuilder("Deleted document >> >> with following ids were deleted as part of GC \n"); >> >> + >> >> Joiner.on(StandardSystemProperty.LINE_SEPARATOR.value()).appendTo(sb, >> >> docIdsToDelete.getIds()); >> >> + log.debug(sb.toString()); >> >> + } >> >> + >> >> + Iterator<List<String>> idListItr = >> >> partition(docIdsToDelete.getIds(), DELETE_BATCH_SIZE); >> >> + while (idListItr.hasNext()) { >> >> + nodeStore.getDocumentStore().remove(Collection.NODES, >> >> idListItr.next()); >> >> + } >> >> + >> >> + nodeStore.invalidateDocChildrenCache(); >> >> + stats.deletedDocGCCount += docIdsToDelete.getSize(); >> >> + } finally { >> >> + docIdsToDelete.close(); >> >> } >> >> - nodeStore.getDocumentStore().remove(Collection.NODES, >> >> docIdsToDelete); >> >> - nodeStore.invalidateDocChildrenCache(); >> >> - stats.deletedDocGCCount += docIdsToDelete.size(); >> >> } >> >> >> >> public static class VersionGCStats { >> >> >> >> Modified: >> >> >> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCDeletionTest.java >> >> URL: >> >> >> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCDeletionTest.java?rev=1666220&r1=1666219&r2=1666220&view=diff >> >> >> >> >> ============================================================================== >> >> --- >> >> >> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCDeletionTest.java >> >> (original) >> >> +++ >> >> >> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCDeletionTest.java >> >> Thu Mar 12 15:22:46 2015 >> >> @@ -36,6 +36,7 @@ import org.junit.Before; >> >> import org.junit.Test; >> >> >> >> import static java.util.concurrent.TimeUnit.HOURS; >> >> +import static org.junit.Assert.assertEquals; >> >> import static org.junit.Assert.assertNull; >> >> import static org.junit.Assert.fail; >> >> >> >> @@ -104,6 +105,53 @@ public class VersionGCDeletionTest { >> >> assertNull(ts.find(Collection.NODES, "1:/x")); >> >> } >> >> >> >> + @Test >> >> + public void deleteLargeNumber() throws Exception{ >> >> + int noOfDocsToDelete = 10000; >> >> + DocumentStore ts = new MemoryDocumentStore(); >> >> + store = new DocumentMK.Builder() >> >> + .clock(clock) >> >> + .setDocumentStore(new MemoryDocumentStore()) >> >> + .setAsyncDelay(0) >> >> + .getNodeStore(); >> >> + >> >> + //Baseline the clock >> >> + clock.waitUntil(Revision.getCurrentTimestamp()); >> >> + >> >> + NodeBuilder b1 = store.getRoot().builder(); >> >> + NodeBuilder xb = b1.child("x"); >> >> + for (int i = 0; i < noOfDocsToDelete; i++){ >> >> + xb.child("a"+i).child("b"+i); >> >> + } >> >> + store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY); >> >> + >> >> + long maxAge = 1; //hours >> >> + long delta = TimeUnit.MINUTES.toMillis(10); >> >> + >> >> + //Remove x/y >> >> + NodeBuilder b2 = store.getRoot().builder(); >> >> + b2.child("x").remove(); >> >> + store.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY); >> >> + >> >> + store.runBackgroundOperations(); >> >> + >> >> + //3. Check that deleted doc does get collected post maxAge >> >> + clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + >> >> delta); >> >> + VersionGarbageCollector gc = >> store.getVersionGarbageCollector(); >> >> + gc.setOverflowToDiskThreshold(100); >> >> + >> >> + VersionGarbageCollector.VersionGCStats stats = gc.gc(maxAge * >> 2, >> >> HOURS); >> >> + assertEquals(noOfDocsToDelete * 2 + 1, >> stats.deletedDocGCCount); >> >> + >> >> + >> >> + assertNull(ts.find(Collection.NODES, "1:/x")); >> >> + >> >> + for (int i = 0; i < noOfDocsToDelete; i++){ >> >> + assertNull(ts.find(Collection.NODES, "2:/a"+i+"/b"+i)); >> >> + assertNull(ts.find(Collection.NODES, "1:/a"+i)); >> >> + } >> >> + } >> >> + >> >> private static class TestDocumentStore extends >> MemoryDocumentStore { >> >> boolean throwException; >> >> @Override >> >> >> >> Modified: >> >> >> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCWithSplitTest.java >> >> URL: >> >> >> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCWithSplitTest.java?rev=1666220&r1=1666219&r2=1666220&view=diff >> >> >> >> >> ============================================================================== >> >> --- >> >> >> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCWithSplitTest.java >> >> (original) >> >> +++ >> >> >> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCWithSplitTest.java >> >> Thu Mar 12 15:22:46 2015 >> >> @@ -137,7 +137,11 @@ public class VersionGCWithSplitTest { >> >> Thread t = new Thread(new Runnable() { >> >> @Override >> >> public void run() { >> >> - stats.set(gc.gc(1, HOURS)); >> >> + try { >> >> + stats.set(gc.gc(1, HOURS)); >> >> + } catch (IOException e) { >> >> + throw new RuntimeException(e); >> >> + } >> >> } >> >> }); >> >> >> >> >> >> >> >> >> > >>
