You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by "Joscorbe (via GitHub)" <gi...@apache.org> on 2026/10/23 07:04:09 UTC
[PR] OAK-12471: Allow multiple paths on oak-run revisions command [jackrabbit-oak]
Joscorbe opened a new pull request, #3227:
URL: https://github.com/apache/jackrabbit-oak/pull/3227
(no comment)
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12471: Allow multiple paths on oak-run revisions command [jackrabbit-oak]
Posted by "Joscorbe (via GitHub)" <gi...@apache.org>.
Joscorbe commented on code in PR #3227:
URL: https://github.com/apache/jackrabbit-oak/pull/3227#discussion_r4380067821
##########
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:
Tests are added
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12471: Allow multiple paths on oak-run revisions command [jackrabbit-oak]
Posted by "Joscorbe (via GitHub)" <gi...@apache.org>.
Joscorbe commented on PR #3227:
URL: https://github.com/apache/jackrabbit-oak/pull/3227#issuecomment-6369022075
Comments are addressed, but the build seems flaky. Will retry later.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12471: Allow multiple paths on oak-run revisions command [jackrabbit-oak]
Posted by "Joscorbe (via GitHub)" <gi...@apache.org>.
Joscorbe commented on code in PR #3227:
URL: https://github.com/apache/jackrabbit-oak/pull/3227#discussion_r4380070576
##########
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:
Corrected
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12471: Allow multiple paths on oak-run revisions command [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12471: Allow multiple paths on oak-run revisions command [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
rishabhdaim commented on code in PR #3227:
URL: https://github.com/apache/jackrabbit-oak/pull/3227#discussion_r4357122539
##########
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()));
Review Comment:
Nitpick: if we have both a path and a file, the path would be picked, and the file would be silently ignored.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12471: Allow multiple paths on oak-run revisions command [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
rishabhdaim commented on code in PR #3227:
URL: https://github.com/apache/jackrabbit-oak/pull/3227#discussion_r4357122539
##########
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()));
Review Comment:
Nitpick: if we have both path and file, path would be picked, and file would be ignored silently.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org