rishabhdaim commented on code in PR #1440: URL: https://github.com/apache/jackrabbit-oak/pull/1440#discussion_r1595018427
########## oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java: ########## @@ -92,9 +90,9 @@ public class RevisionsCommand implements Command { " collect perform garbage collection", " reset clear all persisted metadata", " sweep clean up uncommitted changes", - " detailedGC perform detailed garbage collection i.e. remove unmerged branch commits, old ", + " fullGC perform detailed garbage collection i.e. remove unmerged branch commits, old ", Review Comment: ```suggestion " fullGC perform full garbage collection i.e. remove unmerged branch commits, old ", ``` ########## oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java: ########## @@ -311,10 +311,10 @@ @AttributeDefinition( name = "Document Node Store Detailed GC", description = "Boolean value indicating whether Detailed GC should be enabled for " + Review Comment: ```suggestion description = "Boolean value indicating whether Full GC should be enabled for " + ``` ########## oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java: ########## @@ -748,23 +748,23 @@ private VersionGCStats gc(long maxRevisionAgeInMillis) throws IOException { collectSplitDocuments(phases, sweepRevisions, rec); } } else { - phases.stats.detailedGCDryRunMode = true; + phases.stats.fullGCDryRunMode = true; } // now run detailed GC if enabled - if (detailedGCEnabled) { - stats.detailedGCActive.start(); - if (rec.ignoreDetailedGCDueToCheckPoint) { - phases.stats.ignoredDetailedGCDueToCheckPoint = true; + if (fullGCEnabled) { + stats.fullGCActive.start(); + if (rec.ignoreFullGCDueToCheckPoint) { + phases.stats.ignoredFullGCDueToCheckPoint = true; monitor.skipped("Checkpoint prevented detailed revision garbage collection"); } else { final RevisionVector headRevision = nodeStore.getHeadRevision(); - monitor.info("Looking at revisions in {} for detailed GC", rec.scopeDetailedGC); + monitor.info("Looking at revisions in {} for detailed GC", rec.scopeFullGC); Review Comment: ```suggestion monitor.info("Looking at revisions in {} for full GC", rec.scopeFullGC); ``` ########## oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java: ########## @@ -2243,8 +2242,8 @@ public void testDeletedPropsAndUnmergedBCWithCollisionWithDryRunMode() throws Ex store1.runBackgroundOperations(); // enable the detailed gc flag Review Comment: ```suggestion // enable the Full gc flag ``` ########## oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java: ########## @@ -311,10 +311,10 @@ @AttributeDefinition( name = "Document Node Store Detailed GC", Review Comment: ```suggestion name = "Document Node Store Full GC", ``` ########## oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java: ########## @@ -850,9 +850,9 @@ protected boolean removeEldestEntry(Entry<Path, Boolean> eldest) { lastDoc = doc; // collect the data to delete in next step - if (phases.start(GCPhase.DETAILED_GC_COLLECT_GARBAGE)) { + if (phases.start(GCPhase.FULL_GC_COLLECT_GARBAGE)) { gc.collectGarbage(doc, phases); - phases.stop(GCPhase.DETAILED_GC_COLLECT_GARBAGE); + phases.stop(GCPhase.FULL_GC_COLLECT_GARBAGE); } final Long modified = lastDoc.getModified(); Review Comment: Update below lines: 860 & 862 `collectDetailedGarbage` --> `collectFullGarbage` ########## oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java: ########## @@ -1045,7 +1044,7 @@ public void testGCDeletedNonBundledProps() throws Exception { store1.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY); // enable the detailed gc flag Review Comment: ```suggestion // enable the Full gc flag ``` ########## oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java: ########## @@ -311,10 +311,10 @@ @AttributeDefinition( name = "Document Node Store Detailed GC", description = "Boolean value indicating whether Detailed GC should be enabled for " + - "document node store or not. The Default value is " + DEFAULT_DETAILED_GC_ENABLED + + "document node store or not. The Default value is " + DEFAULT_FULL_GC_ENABLED + ". Note that this value can be overridden via framework " + - "property 'oak.documentstore.detailedGCEnabled'") - boolean detailedGCEnabled() default DEFAULT_DETAILED_GC_ENABLED; + "property 'oak.documentstore.fullGCEnabled'") + boolean fullGCEnabled() default DEFAULT_FULL_GC_ENABLED; @AttributeDefinition( name = "Document Node Store Embedded Verification for Detailed GC", Review Comment: ```suggestion name = "Document Node Store Embedded Verification for Full GC", ``` ########## oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java: ########## @@ -18,6 +18,46 @@ */ package org.apache.jackrabbit.oak.plugins.document; +import com.mongodb.ReadPreference; +import org.apache.jackrabbit.guava.common.base.Function; +import org.apache.jackrabbit.guava.common.base.Predicate; +import org.apache.jackrabbit.guava.common.cache.Cache; +import org.apache.jackrabbit.guava.common.collect.AbstractIterator; +import org.apache.jackrabbit.guava.common.collect.ImmutableList; +import org.apache.jackrabbit.guava.common.collect.Iterators; +import org.apache.jackrabbit.guava.common.collect.Lists; +import org.apache.jackrabbit.guava.common.collect.Queues; +import org.apache.jackrabbit.guava.common.collect.Sets; +import org.apache.jackrabbit.guava.common.util.concurrent.Atomics; +import org.apache.jackrabbit.oak.InitialContent; +import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.plugins.document.DocumentStoreFixture.RDBFixture; +import org.apache.jackrabbit.oak.plugins.document.FailingDocumentStore.FailedUpdateOpListener; +import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.FullGCMode; +import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats; +import org.apache.jackrabbit.oak.plugins.document.bundlor.BundlingConfigInitializer; +import org.apache.jackrabbit.oak.plugins.document.mongo.MongoTestUtils; +import org.apache.jackrabbit.oak.plugins.document.rdb.RDBOptions; +import org.apache.jackrabbit.oak.plugins.document.util.Utils; +import org.apache.jackrabbit.oak.spi.commit.CommitInfo; +import org.apache.jackrabbit.oak.spi.commit.EmptyHook; +import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry; +import org.apache.jackrabbit.oak.spi.state.NodeBuilder; +import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.apache.jackrabbit.oak.stats.Clock; +import org.jetbrains.annotations.NotNull; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; Review Comment: could you please revert the import statement changes? ########## oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java: ########## @@ -1045,7 +1044,7 @@ public void testGCDeletedNonBundledProps() throws Exception { store1.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY); // enable the detailed gc flag Review Comment: Same for all the comment with `// enable the detailed gc flag` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org