OAK-11182: Create and test general mechanism for creating MongoDocumentStore fullGC garbage for testing #1805

shodaaan · 2024-10-17T09:05:34Z

created GenerateFullGCCommand oak-run class with methods for generating garbage of type GAP_ORPHANS and EMPTY_PROPERTIES. All garbage is generated under a specific main node. Also created a cleanup method for deleting all generated garbage
added call to GenerateFullGCCommand to AvailableModes with specific oak-run input variable
created GenerateFullGCCommandTest class with unit test methods for generating and cleaning up garbage
made VersionGarbageCollector.fullGCMode public, so it can be used Command class

…gment has zero length (apache#1427) * OAK-10771 - Log if evicted segment has length zero. * OAK-10771 - Add diskCacheSize stats * OAK-10771 - Record cache size and change --------- Co-authored-by: Axel Hanikel <ahanikel@adobe.com>

…e/deleteMany to make it compatible with both mongo & mongosh

…lters in job summary (apache#1478)

…ache#1466) * OAK-10811: (oak-search-elastic) reduce contention in IndexTracker * OAK-10811: ElasticIndexInfoProvider uses node api to get :status info * OAK-10811: rolled back unneeded change

* OAK-10675: add service principal support in oak-blob-cloud-azure * OAK-10675: use user delegation key signed sas for service principal and add license to new files * OAK-10675: some self review changes * OAK-10675: set null blob headers as empty string * OAK-10675: update pom to resolve package dependency * OAK-10675: add import packages to resolve bundle * OAK-10770: embed runtime dependencies of azure identity in oak-segment-azure * OAK-10770: remove unused imports, refactor dependencies * OAK-10770: add imports, refactor dependencies * OAK-10675: fix runtime dependency issues, code review

…e#1483)

…local names containing '{' or '}'. (apache#1285) Fixed issue and added test case. Co-authored-by: Julian Reschke <reschke@apache.org>

Done --------- Co-authored-by: Julian Sedding <jsedding@apache.org>

* OAK-10780: add azure access token refresh logic * OAK-10780: modify log statement * OAK-10780: modify log statement * OAK-10780: reduce token refresh interval

…che#1486)" This reverts commit 4a4f2ed.

…E in checkpoints (apache#1487)

…7.1 (apache#1486)"" This reverts commit a9d0c1c.

…anded names (apache#1488) * OAK-10621: o.a.j.o.namepath.JcrPathParser does not accept indexed expanded names. Fixed, added unit tests.

…n an indexing phase fails. (apache#1481)

…ine check (apache#1491)

done

* OAK-10675: add service principal support in oak-blob-cloud-azure * OAK-10675: use user delegation key signed sas for service principal and add license to new files * OAK-10675: some self review changes * OAK-10675: set null blob headers as empty string * OAK-10675: update pom to resolve package dependency * OAK-10675: add import packages to resolve bundle * OAK-10770: embed runtime dependencies of azure identity in oak-segment-azure * OAK-10770: remove unused imports, refactor dependencies * OAK-10770: add imports, refactor dependencies * OAK-10675: fix runtime dependency issues, code review * OAK-10781: add access token refresh mechanism * close executor * reduce token refresh delauy to 1 minute * retrigger build

…#1454) Co-authored-by: Stefan Egli <stefanegli@apache.org> Co-authored-by: Rishabh Kumar <diam@adobe.com> Co-authored-by: Jose Cordero <corderob@adobe.com> Co-authored-by: Daniel Iancu <iancu@adobe.com> Full details in branch: DetailedGC/OAK-10199

…p narrow down where time is spent

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

…generated garbage nodes to shorter names - made executor a class level variable and added close method for it - renamed files to include "Garbage" - other small suggested changes

…arbageOptionType parameter passed to the generateGarbage Command - reverted change in VersionGarbageCollector.java

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

oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/GenerateGarbageHelper.java

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

…ommand, which allow for generating gap orphans up to 15 levels deep and also allows for deleting nodes in their hierarchy up to the specified orphansLevelGap level - separated the garbage generation code for EMPTY_PROPS / GAP_ORPHANS into 2 separate methods - adjusted existing unit test methods in GenerateGarbageCommandTest and added new ones for the new level / gap parameters - other small changes from the PR review

stefan-egli

Added a few more points.

One general question is : as this command is clearly a test command not to be used against a production environment : how could that be emphasized. For example: we could add an explicit: "are you really sure you want to generate test garbage against ...". That would make it clear that the target repository is going to be written to. Wdyt?

oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/GenerateGarbageHelper.java

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

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

…der the garbage root path - added additional unit test for cleanup of empty properties garbage - used path utility methods where possible - added some method descriptions where relevant - renamed constants and updated comments and strings to remove "fullGC" from strings

shodaaan · 2024-10-29T08:01:40Z

Added a few more points.

One general question is : as this command is clearly a test command not to be used against a production environment : how could that be emphasized. For example: we could add an explicit: "are you really sure you want to generate test garbage against ...". That would make it clear that the target repository is going to be written to. Wdyt?

I resolved all of the points, thank you for the suggestions.

Regarding the explicit check, I don't think this is necessary because oak-run is a tool that by design makes important / deep changes in the MongoDB it is run on, so users should always be careful and know the exact consequences of the commands they run.
The USAGE text clearly states what the command and associated subcommands do.
Also, since oak-run commands are designed to be used "with intent", and not "by accident", having an additional confirmation step for each garbage generation command run would just annoy the user IMHO. I didn't see this check for any other command in oak-run.

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

stefan-egli · 2024-10-29T11:25:31Z

Regarding the explicit check, I don't think this is necessary

Fair enough.

Two unrelated additional comments:

the DocumentNodeStore is not properly disposed. You can see this by running the oak-run command consecutively : the second start will have to wait 2min for the lease timeout of the first non-gracefully shut down run. We should ensure a graceful disposal.
what might be useful is to add the base path where the garbage is generated to - such that the user immediately knows (and doesn't eg have to consult the source code)

- added documentNodeStore.dispose() calls to CreateGarbageCommand.java

shodaaan · 2024-10-29T13:41:13Z

Regarding the explicit check, I don't think this is necessary

Fair enough.

Two unrelated additional comments:

the DocumentNodeStore is not properly disposed. You can see this by running the oak-run command consecutively : the second start will have to wait 2min for the lease timeout of the first non-gracefully shut down run. We should ensure a graceful disposal.

what might be useful is to add the base path where the garbage is generated to - such that the user immediately knows (and doesn't eg have to consult the source code)

I added documentNodeStore.dispose() calls to CreateGarbageCommand. Thank you for the suggestion

The root path where garbage is generated (/tmp/oak-run-generated-test-garbage) is specified in the USAGE string of CreateGarbageCommand, so users will know what the path is where garbage is generated.

- added disabled lease check mode setting to DocumentNodeStoreBuilder in GenerateGarbageCommand - added some more output messages to make the functionality clearer when running oak-run

shodaaan · 2024-11-06T16:03:30Z

Regarding the explicit check, I don't think this is necessary

Fair enough.
Two unrelated additional comments:

the DocumentNodeStore is not properly disposed. You can see this by running the oak-run command consecutively : the second start will have to wait 2min for the lease timeout of the first non-gracefully shut down run. We should ensure a graceful disposal.

what might be useful is to add the base path where the garbage is generated to - such that the user immediately knows (and doesn't eg have to consult the source code)

I added documentNodeStore.dispose() calls to CreateGarbageCommand. Thank you for the suggestion

The root path where garbage is generated (/tmp/oak-run-generated-test-garbage) is specified in the USAGE string of CreateGarbageCommand, so users will know what the path is where garbage is generated.

I added proper disposal of DocumentNodeStore for both createGarbage and cleanupGarbage commands. Tested oak-run with repeated runs and disposal works correctly.

I also added some println messages with the base path of the generated garbage, as well as the paths of the documents deleted by the "clean" sub-command.

oak-run/src/test/java/org/apache/jackrabbit/oak/plugins/document/CreateGarbageCommandTest.java

oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/GenerateGarbageHelper.java

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

rishabhdaim

I would replace the usage of generate with create

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

- removed Closeable implementation of CreateGarbageCommand - some minor renames

stefan-egli

Had troubles getting the continuous mode running and I believe it's broken because of the new closer logic closing it early.

(other than that added a few detail remarks as well)

oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/FullGCHelper.java

oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/CreateGarbageHelper.java

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

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

…d.java in order to cover all cases, including continuous garbage generation - updated resource disposal in unit tests to account for changes - other minor changes / renames

shodaaan · 2024-11-12T16:34:02Z

Had troubles getting the continuous mode running and I believe it's broken because of the new closer logic closing it early.

(other than that added a few detail remarks as well)

Tested oak-run with all modes (including continuous generation), everything works fine now.

stefan-egli

+1 lgtm, started the jenkins run, would suggest to go ahead with merge unless we find a regression (unlikely), and then we can iterate from there

stefan-egli · 2024-11-14T13:55:46Z

would suggest to go ahead with merge unless we find a regression

squash merge obviously!

Rishabh Kumar and others added 30 commits June 13, 2024 17:59

OAK-10813 : using projection while listing/removing root temp nodes

c0ba6b3

OAK-10813 : using projection while listing children of an node

2bdeb41

OAK-10824: Release Oak 1.64.0 - Candidate Release Notes

a3d5a17

OAK-10824: Release Oak 1.64.0 - Candidate Release Notes

ceb24ff

[maven-release-plugin] prepare release jackrabbit-oak-1.64.0

dd52a97

[maven-release-plugin] prepare for next development iteration

568348f

OAK-10823 : replaced db.collection.remove with db.collection.deleteOn…

b3cf8fd

…e/deleteMany to make it compatible with both mongo & mongosh

OAK-10823 : reverted un-intended changes from var to let/const

d29d7bb

OAK-10789 - Log paths used for inclusing/exclusion for Mongo regex fi…

a05caf3

…lters in job summary (apache#1478)

OAK-10811: (oak-search-elastic) reduce contention in IndexTracker (ap…

71742bf

…ache#1466) * OAK-10811: (oak-search-elastic) reduce contention in IndexTracker * OAK-10811: ElasticIndexInfoProvider uses node api to get :status info * OAK-10811: rolled back unneeded change

OAK-10829: examples: update build-helper-maven-plugin to 3.6.0 (apach…

4546f5b

…e#1483)

OAK-10624: o.a.j.o.namepath.JcrPathParser does not accept some valid …

3066e50

…local names containing '{' or '}'. (apache#1285) Fixed issue and added test case. Co-authored-by: Julian Reschke <reschke@apache.org>

OAK-6756: Convert oak-auth-external to OSGi R7 annotations (apache#1371)

6d6b836

Done --------- Co-authored-by: Julian Sedding <jsedding@apache.org>

OAK-10780: add azure access token refresh logic (apache#1441)

dc21de4

* OAK-10780: add azure access token refresh logic * OAK-10780: modify log statement * OAK-10780: modify log statement * OAK-10780: reduce token refresh interval

OAK-10830: Upgrade jackson-databind dependency to 2.17.1 (apache#1486)

96f09e0

OAK-10826 - Misleading warning about missing checkpoint

81ba45b

Revert "OAK-10830: Upgrade jackson-databind dependency to 2.17.1 (apa…

48cff76

…che#1486)" This reverts commit 4a4f2ed.

OAK-10833 - Consistency check reports success for repository with SNF…

d4cb26a

…E in checkpoints (apache#1487)

Revert "Revert "OAK-10830: Upgrade jackson-databind dependency to 2.1…

a3f0a07

…7.1 (apache#1486)"" This reverts commit a9d0c1c.

OAK-10621: o.a.j.o.namepath.JcrPathParser does not accept indexed exp…

c92e2d1

…anded names (apache#1488) * OAK-10621: o.a.j.o.namepath.JcrPathParser does not accept indexed expanded names. Fixed, added unit tests.

OAK-10824: Release Oak 1.64.0

f30e86f

OAK-10825 - Indexing job: log [TASK:<PHASE>:FAIL] log message for whe…

5398ee4

…n an indexing phase fails. (apache#1481)

OAK-10838: oak-lucene: maven install fails in oak-lucene due to basel…

e50196f

…ine check (apache#1491)

OAK-6761: Convert oak-blob-plugins to OSGi R7 annotations (apache#1415)

b39d09d

done

OAK-10843 : temporarily disable flaky tests

6215e57

OAK-10844 : removing some test combinations and adding logging to hel…

bb8c214

…p narrow down where time is spent

rishabhdaim requested changes Oct 21, 2024

View reviewed changes

- modified generated garbage path to be under "tmp" node and renamed …

f496ea8

…generated garbage nodes to shorter names - made executor a class level variable and added close method for it - renamed files to include "Garbage" - other small suggested changes

shodaaan requested a review from rishabhdaim October 21, 2024 15:16

shodaaan added 2 commits October 21, 2024 18:18

- replaced FULLGC with GARBAGE in constant names

54887a0

- created GenerateGarbageHelper class with methods for checking the g…

15a9e3c

…arbageOptionType parameter passed to the generateGarbage Command - reverted change in VersionGarbageCollector.java

rishabhdaim requested changes Oct 22, 2024

View reviewed changes

stefan-egli reviewed Oct 28, 2024

View reviewed changes

rishabhdaim reviewed Oct 29, 2024

View reviewed changes

oak-run/src/main/java/org/apache/jackrabbit/oak/run/AvailableModes.java Outdated Show resolved Hide resolved

- renamed command to "create-test-garbage"

9d9a349

- renamed "Generate" to "Create" in class names and relevant methods

39b8863

- added documentNodeStore.dispose() calls to CreateGarbageCommand.java

- added documentNodeStore.dispose() calls to CreateGarbageCommand.java

65f94c5

- added disabled lease check mode setting to DocumentNodeStoreBuilder in GenerateGarbageCommand - added some more output messages to make the functionality clearer when running oak-run