rishabhdaim commented on code in PR #3227:
URL: https://github.com/apache/jackrabbit-oak/pull/3227#discussion_r4357269566
##########
oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:
##########
@@ -671,16 +691,30 @@ private void collectDocument(RevisionsOptions options,
Closer closer, String pat
gc.setGCMonitor(new LoggingGCMonitor(LOG));
// Set a default statistics provider
gc.setStatisticsProvider(new
DefaultStatisticsProvider(Executors.newSingleThreadScheduledExecutor()));
- // Run fullGC on the given document
- NodeDocument workingDocument = documentStore.find(NODES,
getIdFromPath(path));
- if (workingDocument == null) {
- System.err.println("Document not found: " + path);
- return;
- }
- gc.collectGarbageOnDocument(documentNodeStore, workingDocument,
options.isVerbose());
+ long start = System.currentTimeMillis();
+ int notFound = 0;
+ for (int i = 0; i < paths.size(); i++) {
+ String path = paths.get(i);
+ long elapsed = System.currentTimeMillis() - start;
+ long etaMs = i == 0 ? 0 : elapsed * (paths.size() - i) / i;
+ System.out.printf("[%d/%d] elapsed %s, ETA %s - running fullGC on
the document: %s%n",
+ i + 1, paths.size(), fmtDuration(elapsed),
fmtDuration(etaMs), path);
+ // Run fullGC on the given document
+ NodeDocument workingDocument = documentStore.find(NODES,
getIdFromPath(path));
+ if (workingDocument == null) {
+ System.err.println("Document not found: " + path);
+ notFound++;
+ continue;
Review Comment:
๐ก **P3** ยท ๐ฏ Correctness
**๐ Issue:** No test invokes `--pathsFile`, so the new parser and mixed
missing/present document loop are untested.
**๐ ๏ธ Fix:** Add command tests using a temporary UTF-8 file with multiple
existing paths, a missing path and blank lines; assert that processing
continues after the missing entry and that the summary counts are correct.
Cover empty files and precedence with `--path`/`--entireRepo` as well.
##########
oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:
##########
@@ -362,9 +374,11 @@ public void execute(String... args) throws Exception {
if (entireRepo) {
collect(options, closer, true);
} else if (isNotEmpty(path)) {
- collectDocument(options, closer, path);
+ collectDocuments(options, closer,
Collections.singletonList(path));
+ } else if (isNotEmpty(options.getPathsFile())) {
+ collectDocuments(options, closer,
readPaths(options.getPathsFile()));
} else {
- System.err.println("--path or --entireRepo option is
required for " +
+ System.err.println("--path, --pathsFile or --entireRepo
option is required for " +
Review Comment:
๐ **P2** ยท ๐ฏ Correctness
**๐ Issue:** `fullGCWithEmbeddedWithoutEntireRepoOrPath` still asserts the
old `--path or --entireRepo` diagnostic. This message now includes
`--pathsFile`, so the existing test fails when its MongoDB prerequisite is
satisfied.
**๐ ๏ธ Fix:** Update the assertion to expect all three selectors and run that
targeted test with MongoDB available.
--
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]
