stefan-egli commented on code in PR #993:
URL: https://github.com/apache/jackrabbit-oak/pull/993#discussion_r1234124586


##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java:
##########
@@ -521,6 +593,90 @@ private VersionGCStats gc(long maxRevisionAgeInMillis) 
throws IOException {
             return stats;
         }
 
+        /**
+         * "Detail garbage" refers to additional garbage identified as part of 
OAK-10199
+         * et al: essentially garbage that in earlier versions of Oak were 
ignored. This
+         * includes: deleted properties, revision information within 
documents, branch
+         * commit related garbage.
+         * <p/>
+         * TODO: limit this to run only on a singleton instance, eg the 
cluster leader
+         * <p/>
+         * The "detail garbage" collector can be instructed to do a full 
repository scan
+         * - or incrementally based on where it last left off. When doing a 
full
+         * repository scan (but not limited to that), it executes in (small) 
batches
+         * followed by voluntary paused (aka throttling) to avoid excessive 
load on the
+         * system. The full repository scan does not have to finish 
particularly fast,
+         * it is okay that it takes a considerable amount of time.
+         *
+         * @param phases {@link GCPhases}
+         * @param headRevision the current head revision of node store
+         */
+        private void collectDetailedGarbage(final GCPhases phases, final 
RevisionVector headRevision, final VersionGCRecommendations rec)
+                throws IOException {
+            int docsTraversed = 0;
+            boolean foundDoc = true;
+            long oldestModifiedDocTimeStamp = rec.scopeDetailedGC.fromMs;
+            String oldestModifiedDocId = rec.detailedGCId;
+            try (DetailedGC gc = new DetailedGC(headRevision, monitor, 
cancel)) {
+                final long fromModified = rec.scopeDetailedGC.fromMs;
+                final long toModified = rec.scopeDetailedGC.toMs;
+                if (phases.start(GCPhase.DETAILED_GC)) {
+                    while (foundDoc && oldestModifiedDocTimeStamp < toModified 
&& docsTraversed <= PROGRESS_BATCH_SIZE) {
+                        // set foundDoc to false to allow exiting the while 
loop
+                        foundDoc = false;
+                        Iterable<NodeDocument> itr = 
versionStore.getModifiedDocs(oldestModifiedDocTimeStamp, toModified, 1000, 
oldestModifiedDocId);

Review Comment:
   With the recent change of using `oldestModifiedDocId` here, this iterator 
now misses that document. Eg if `oldestModifiedDocId="0:/"`, then the 
underlying call to `DocumentStore.query` has `"0:/"` as the `fromId`, but the 
javadoc of `DocumentStore.query` states that the 
[fromId](https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentStore.java#L126)
 is excluded.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to