OAK-10792 - Rename DetailedGC to FullGC #1440

daniancu · 2024-05-08T07:26:32Z

Switching to FullGC instead of DetailedGC everywhere, method names, constants, arguments etc

reschke · 2024-05-08T15:20:50Z

Can you please open a JIRA ticket to track this change?

reschke · 2024-05-08T15:22:17Z

oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java

@@ -46,19 +31,32 @@
 import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
 import org.apache.jackrabbit.oak.plugins.document.RevisionContextWrapper;
 import org.apache.jackrabbit.oak.plugins.document.SweepHelper;
+import org.apache.jackrabbit.oak.plugins.document.VersionGCOptions;


Please do not move imports around (unless the change is exactly for thar purpose)

sorry for that, I've set them back

daniancu · 2024-05-09T10:10:34Z

Can you please open a JIRA ticket to track this change?

https://issues.apache.org/jira/browse/OAK-10792

daniancu · 2024-05-10T10:42:32Z

@reschke @rishabhdaim can you take an other look?

rishabhdaim · 2024-05-09T06:22:52Z

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 ",


Suggested change

" fullGC perform detailed garbage collection i.e. remove unmerged branch commits, old ",

" fullGC perform full garbage collection i.e. remove unmerged branch commits, old ",

rishabhdaim · 2024-05-09T06:25:16Z

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 " +


Suggested change

description = "Boolean value indicating whether Detailed GC should be enabled for " +

description = "Boolean value indicating whether Full GC should be enabled for " +

rishabhdaim · 2024-05-09T06:25:25Z

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",


Suggested change

name = "Document Node Store Detailed GC",

name = "Document Node Store Full GC",

rishabhdaim · 2024-05-13T10:06:31Z

oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java

-                    "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",


Suggested change

name = "Document Node Store Embedded Verification for Detailed GC",

name = "Document Node Store Embedded Verification for Full GC",

rishabhdaim · 2024-05-13T10:09:40Z

...cument/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java

                        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);


Suggested change

monitor.info("Looking at revisions in {} for detailed GC", rec.scopeFullGC);

monitor.info("Looking at revisions in {} for full GC", rec.scopeFullGC);

rishabhdaim · 2024-05-13T10:12:55Z

...cument/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java

                                    gc.collectGarbage(doc, phases);
-                                    phases.stop(GCPhase.DETAILED_GC_COLLECT_GARBAGE);
+                                    phases.stop(GCPhase.FULL_GC_COLLECT_GARBAGE);
                                }

                                final Long modified = lastDoc.getModified();


Update below lines: 860 & 862

collectDetailedGarbage --> collectFullGarbage

rishabhdaim · 2024-05-13T10:17:01Z

...ment/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java

+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;


could you please revert the import statement changes?

rishabhdaim · 2024-05-13T10:18:12Z

...ment/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


Suggested change

// enable the detailed gc flag

// enable the Full gc flag

Same for all the comment with // enable the detailed gc flag

@rishabhdaim I think I fixed them all, can you review pls?

rishabhdaim · 2024-05-13T10:19:15Z

...ment/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


Suggested change

// enable the detailed gc flag

// enable the Full gc flag

rishabhdaim · 2024-05-13T10:22:30Z

We should only merge this PR after incorporating the integration branch changes into the trunk.
cc @stefan-egli @reschke

stefan-egli · 2024-05-13T14:43:09Z

We should only merge this PR after incorporating the integration branch changes into the trunk.

@rishabhdaim does it otherwise cause merge conflicts? I understand the concern - on the other hand this should only change new code, so wouldn't expect additional merge conflicts? (but if it would, then yes)

stefan-egli · 2024-05-14T14:45:14Z

@daniancu , FTR, we should prefix the commit message with the jira ticket (agreed upon practice, helps navigate commits)

Joscorbe requested review from stefan-egli, rishabhdaim and Joscorbe May 8, 2024 14:44

reschke reviewed May 8, 2024

View reviewed changes

daniancu force-pushed the DetailedGC/OAK-10199 branch from 8d858d1 to b34ad4b Compare May 9, 2024 08:20

daniancu changed the title ~~Rename DetailedGC to FullGC~~ OAK-10792 - Rename DetailedGC to FullGC May 9, 2024

daniancu requested a review from reschke May 9, 2024 09:56

rishabhdaim requested changes May 13, 2024

View reviewed changes

Rename DetailedGC to FullGC

a36514f

daniancu force-pushed the DetailedGC/OAK-10199 branch from b34ad4b to a36514f Compare May 13, 2024 13:50

daniancu requested a review from rishabhdaim May 13, 2024 14:21

rishabhdaim approved these changes May 14, 2024

View reviewed changes

rishabhdaim merged commit 9e33faf into apache:DetailedGC/OAK-10199 May 14, 2024
1 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OAK-10792 - Rename DetailedGC to FullGC #1440

OAK-10792 - Rename DetailedGC to FullGC #1440

daniancu commented May 8, 2024

reschke commented May 8, 2024

reschke May 8, 2024

daniancu May 9, 2024

daniancu commented May 9, 2024

daniancu commented May 10, 2024

rishabhdaim May 9, 2024

daniancu May 13, 2024

rishabhdaim May 9, 2024

daniancu May 13, 2024

rishabhdaim May 9, 2024

daniancu May 13, 2024

rishabhdaim May 13, 2024

daniancu May 13, 2024

rishabhdaim May 13, 2024

daniancu May 13, 2024

rishabhdaim May 13, 2024

daniancu May 13, 2024

rishabhdaim May 13, 2024

daniancu May 13, 2024

rishabhdaim May 13, 2024

rishabhdaim May 13, 2024

daniancu May 13, 2024

rishabhdaim May 13, 2024

daniancu May 13, 2024

rishabhdaim commented May 13, 2024

stefan-egli commented May 13, 2024

stefan-egli commented May 14, 2024

	" fullGC perform detailed garbage collection i.e. remove unmerged branch commits, old ",
	" fullGC perform full garbage collection i.e. remove unmerged branch commits, old ",

	description = "Boolean value indicating whether Detailed GC should be enabled for " +
	description = "Boolean value indicating whether Full GC should be enabled for " +

		@@ -311,10 +311,10 @@
		@AttributeDefinition(
		name = "Document Node Store Detailed GC",

	name = "Document Node Store Detailed GC",
	name = "Document Node Store Full GC",

	name = "Document Node Store Embedded Verification for Detailed GC",
	name = "Document Node Store Embedded Verification for Full GC",

	monitor.info("Looking at revisions in {} for detailed GC", rec.scopeFullGC);
	monitor.info("Looking at revisions in {} for full GC", rec.scopeFullGC);

		@@ -1045,7 +1044,7 @@ public void testGCDeletedNonBundledProps() throws Exception {
		store1.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);

		// enable the detailed gc flag

		@@ -2243,8 +2242,8 @@ public void testDeletedPropsAndUnmergedBCWithCollisionWithDryRunMode() throws Ex
		store1.runBackgroundOperations();

		// enable the detailed gc flag

OAK-10792 - Rename DetailedGC to FullGC #1440

OAK-10792 - Rename DetailedGC to FullGC #1440

Conversation

daniancu commented May 8, 2024

reschke commented May 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniancu commented May 9, 2024

daniancu commented May 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rishabhdaim commented May 13, 2024

stefan-egli commented May 13, 2024

stefan-egli commented May 14, 2024