You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by da...@apache.org on 2023/06/19 08:15:43 UTC

[jackrabbit-oak] branch OAK-10199 updated (aa504ba672 -> 580cd8e414)

This is an automated email from the ASF dual-hosted git repository.

daim pushed a change to branch OAK-10199
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git


    omit aa504ba672 OAK-10199 : fixed code smells as suggested by Sonar
    omit 1dc031bad0 OAK-10199 : ignore documents which doesn't have _modified field in mongo while fetching modifiedDocs
    omit bea04c299c OAK-10199 : used bulk findAndModify api to perform garbage cleanup
    omit 4b68776807 OAK-10199 : override getModifiedDocs() for RDB and added unit cases for deletedProps
    omit 4b5722154e OAK-10199 : provided support for feature toggle & osgi config for detailed gc
    omit 29493dca98 OAK-10199 : disable the detailGc in tearDown to avoid side-effects
    omit 713fae34c3 OAK-10199 : initial sketch of detail gc skeleton
     add 846fcb4e89 OAK-10265 | Refresh the index def after lane revert in oak-run out of… (#958)
     add 353a3f4ce7 OAK-10263 Prevent inconsistent state in TarWriter (#957)
     add e384b80d34 OAK-10270 : document limitation in default perm evaluation regarding TreePermissionImpl.canRead(PropertyState)
     add 0ccdf7ea8e OAK-10271 : Fix dependencies on oak-exercise
     add 7db2a795aa OAK-10267: make elastic indexes more lenient when fields cannot be converted (#961)
     add dd6d968c80 OAK-10252: Distinguish between provider and consumer type interfaces (#948)
     add 0e7ff58d14 OAK-10275: oak-upgrade: remove workaround for Java 1.6 (#966)
     add 72f5e0729a OAK-10276: switch oak-upgrade to shaded guava (#965)
     add 1674cd82a9 OAK-10278: switch oak-run-elastic to shaded guava (#967)
     add aa3c8d6ef2 OAK-10282: switch oak-it to shaded guava (#968)
     add 1909b7cc06 OAK-10283: switch oak-examples to shaded guava (#970)
     add e503b50d9d OAK-10284: switch oak-benchmarks to shaded guava (#972)
     add 142e4be8f7 OAK-10286 : AutoMembershipPrincipals.isInheritedMember add check for cyclic membership, OAK-10285 : MembershipProvider change log level to ERROR for cyclic membership (#971)
     add 9f3358d11a OAK-9660: NullPointerException When Moving Transient node
     add 488cca50af Merge pull request #461 from mreutegg/OAK-9660
     add 136bf146f5 OAK-10287: switch oak-benchmarks-lucene to shaded guava (#973)
     add 5ae2e6c5d2 OAK-10280: Occasional failure to start docker container
     add 3a9d407841 OAK-10280: Occasional failure to start docker container
     add f49e2caccb OAK-10280: Occasional failure to start docker container
     add de3c2755f9 Merge pull request #969 from mreutegg/OAK-10280
     add dea494cc4b OAK-10268: propertyIndex=false fields cannot be used for sorting (#963)
     add 3796f984ab OAK-10290: switch oak-benchmarks-elastic to shaded guava (#976)
     add 04c23cb1da OAK-10292: switch oak-benchmarks-solr to shaded guava (#978)
     add 7bcb56a077 OAK-10291: oak-segment-remote: PersistentRedisCacheTest may fail on Windows due to insufficient pagefile size. (#977)
     add bc5150dfc8 OAK-10147: Many move operations may consume a lot of memory
     add 2a42d09b01 OAK-10147: Many move operations may consume a lot of memory
     add 32620faa29 OAK-10147: Many move operations may consume a lot of memory
     add d4df2711e0 OAK-10147: Many move operations may consume a lot of memory
     add e1f98641b7 OAK-10147: Many move operations may consume a lot of memory
     add 50586200f7 OAK-10147: Many move operations may consume a lot of memory
     add a520e5de6f Merge pull request #962 from mreutegg/OAK-10147-4
     add 9186a659c3 OAK-9660: NullPointerException When Moving Transient node
     add 22e72d6b5a Merge pull request #975 from mreutegg/OAK-9660-1
     add c6d7ee8691 OAK-10297: Update (shaded) Guava to 32.0.1 (#980)
     add b36f96adcf OAK-10247: oak-commons: remove Guava from public API (#944)
     add 3320982566 OAK-10301: Update Mockito dependency to 4.11.0 (#983)
     add d3b9502fe3 NOJIRA: remove propertyIndex/nodeScopeIndex from dynamicBoost test to verify they still succeed (#974)
     add ff0bf210fc OAK-10305: make annotation dependencies have scope "provided" (#984)
     add b8b7c35df8 OAK-10247: oak-commons: remove Guava from public API - remove remaining uses and add to shaded version to Profiler ignore list (#986)
     add afddcd7251 OAK-10306: Incorrect dependency scope for commons-math3
     add f3f8059087 Merge pull request #985 from mreutegg/OAK-10306
     add 1d6581bb10 OAK-10307: oak-shaded-guava leaks original guava as transitive dependency (#989)
     add a02cbb9d0b OAK-10307: exclude oak-shaded-guava/dependency-reduced-pom.xml from rat plugin check
     add 8dc7950337 Revert "OAK-10307: exclude oak-shaded-guava/dependency-reduced-pom.xml from rat plugin check"
     add 91d10c9f71 Revert "OAK-10307: oak-shaded-guava leaks original guava as transitive dependency (#989)"
     new 6e25ec594d OAK-10199 : initial sketch of detail gc skeleton
     new fdee87a36c OAK-10199 : disable the detailGc in tearDown to avoid side-effects
     new f0dcb48535 OAK-10199 : provided support for feature toggle & osgi config for detailed gc
     new 202dd8a9fb OAK-10199 : override getModifiedDocs() for RDB and added unit cases for deletedProps
     new 2431421d11 OAK-10199 : used bulk findAndModify api to perform garbage cleanup
     new 1d47af5a84 OAK-10199 : ignore documents which doesn't have _modified field in mongo while fetching modifiedDocs
     new 2a330be5e4 OAK-10199 : fixed code smells as suggested by Sonar
     new 580cd8e414 OAK-10199 : updated logic to fetch nodes by sorting them on the basis of _modified & _id

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (aa504ba672)
            \
             N -- N -- N   refs/heads/OAK-10199 (580cd8e414)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 8 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../impl/principal/AutoMembershipPrincipals.java   |  12 +-
 .../impl/principal/AutoMembershipCycleTest.java    | 109 +++++++++++
 ...lasticFullTextWithoutGlobalIndexSearchTest.java |   2 +-
 .../benchmark/util/ElasticGlobalInitializer.java   |   2 +-
 .../org/apache/jackrabbit/oak/run/ElasticMain.java |   2 +-
 .../jackrabbit/oak/benchmark/HybridIndexTest.java  |  10 +-
 ...LuceneFullTextWithoutGlobalIndexSearchTest.java |   2 +-
 ...LucenePropertyFTIndexedContentAvailability.java |   2 +-
 ...pertyFTSeparatedIndexedContentAvailability.java |   2 +-
 .../oak/benchmark/LucenePropertySearchTest.java    |   2 +-
 .../oak/benchmark/util/OakLuceneIndexUtils.java    |   4 +-
 .../org/apache/jackrabbit/oak/run/LuceneMain.java  |   2 +-
 .../benchmarks/search/AggregateNodeSearcher.java   |   2 +-
 .../suites/ScalabilityBlobSearchSuite.java         |  10 +-
 .../suites/ScalabilityNodeRelationshipSuite.java   |  10 +-
 .../scalability/suites/ScalabilityNodeSuite.java   |  14 +-
 .../org/apache/jackrabbit/oak/run/SolrMain.java    |   2 +-
 .../jackrabbit/oak/benchmark/AbstractTest.java     |   2 +-
 ...ringWriteTest.java => AccessAfterMoveTest.java} |  41 ++--
 .../jackrabbit/oak/benchmark/BenchmarkRunner.java  |   9 +-
 .../jackrabbit/oak/benchmark/BundlingNodeTest.java |   4 +-
 .../oak/benchmark/CompositeAuthorizationTest.java  |   2 +-
 .../oak/benchmark/ConcurrentFileWriteTest.java     |   2 +-
 .../oak/benchmark/ConcurrentHasPermissionTest.java |   2 +-
 .../oak/benchmark/ContinuousRevisionGCTest.java    |   2 +-
 .../apache/jackrabbit/oak/benchmark/CugTest.java   |   2 +-
 .../jackrabbit/oak/benchmark/FacetSearchTest.java  |   8 +-
 .../IsCheckedOutAddMixinSetPropertyTest.java       |   2 +-
 .../jackrabbit/oak/benchmark/LoginSystemTest.java  |   2 +-
 .../jackrabbit/oak/benchmark/MemberBaseTest.java   |   2 +-
 .../jackrabbit/oak/benchmark/ObservationTest.java  |   2 +-
 .../oak/benchmark/PropertyFullTextTest.java        |   4 +-
 .../jackrabbit/oak/benchmark/ReadManyTest.java     |   2 +-
 .../jackrabbit/oak/benchmark/ReadPropertyTest.java |   2 +-
 .../jackrabbit/oak/benchmark/RevisionGCTest.java   |   2 +-
 .../jackrabbit/oak/benchmark/SearchTest.java       |   6 +-
 .../jackrabbit/oak/benchmark/SetPropertyTest.java  |   2 +-
 .../oak/benchmark/SmallFileReadTest.java           |   2 +-
 .../external/AbstractExternalTest.java             |   6 +-
 .../authentication/external/ExternalLoginTest.java |   2 +-
 .../external/PrincipalNameResolutionTest.java      |   2 +-
 .../authorization/AbstractHasItemGetItemTest.java  |   4 +-
 .../benchmark/authorization/AceCreationTest.java   |   2 +-
 .../GetPrivilegeCollectionIncludeNamesTest.java    |   4 +-
 .../HasPermissionHasItemGetItemTest.java           |   2 +-
 .../permission/EagerCacheSizeTest.java             |   4 +-
 .../principalbased/PermissionEvaluationTest.java   |   2 +-
 .../principalbased/PrinicipalBasedReadTest.java    |  12 +-
 .../oak/benchmark/util/OakIndexUtils.java          |   4 +-
 .../oak/benchmark/wikipedia/WikipediaImport.java   |   2 +-
 .../java/org/apache/jackrabbit/oak/run/Main.java   |   2 +-
 .../oak/scalability/ScalabilityRunner.java         |  10 +-
 .../benchmarks/search/ConcurrentReader.java        |   2 +-
 .../standby/StandbyBulkTransferBenchmark.java      |   2 +-
 .../suites/ScalabilityAbstractSuite.java           |  12 +-
 .../cloud/azure/blobstorage/AzuriteDockerRule.java |  29 ++-
 oak-commons/pom.xml                                |  12 +-
 .../apache/jackrabbit/oak/commons/FileIOUtils.java | 212 --------------------
 .../jackrabbit/oak/commons/GuavaDeprecation.java   |  65 ------
 .../apache/jackrabbit/oak/commons/Profiler.java    |   1 +
 .../oak/commons/io/BurnOnCloseFileIterator.java    |   3 +-
 .../jackrabbit/oak/commons/io/LazyInputStream.java |  15 +-
 .../jackrabbit/oak/commons/io/package-info.java    |   2 +-
 .../jackrabbit/oak/commons/package-info.java       |   2 +-
 .../jackrabbit/oak/commons/FileIOUtilsTest.java    | 101 +---------
 .../commons/FileLineDifferenceIteratorTest.java    |  43 +---
 .../oak/commons/io/LazyInputStreamTest.java        |  81 --------
 oak-core/pom.xml                                   |   2 +
 .../main/java/org/apache/jackrabbit/oak/Oak.java   |   6 +-
 .../jackrabbit/oak/core/ContentRepositoryImpl.java |  11 +-
 .../jackrabbit/oak/core/ContentSessionImpl.java    |   9 +-
 .../apache/jackrabbit/oak/core/MutableRoot.java    | 125 ++++++++++--
 .../org/apache/jackrabbit/oak/core/SystemRoot.java |   4 +-
 .../oak/plugins/index/IndexConstants.java          |   5 +
 .../plugins/index/importer/AsyncLaneSwitcher.java  |   2 +
 .../oak/security/user/MembershipProvider.java      |   2 +-
 .../org/apache/jackrabbit/oak/core/MoveTest.java   | 218 +++++++++++++++++----
 .../jackrabbit/oak/core/MutableRootTest.java       |   3 +-
 .../site/markdown/security/permission/default.md   |  45 +++--
 .../oak/standalone/RepositoryInitializer.java      |   6 +-
 .../java/org/apache/jackrabbit/j2ee/TomcatIT.java  |   2 +-
 oak-exercise/pom.xml                               |  23 +++
 .../java/org/apache/jackrabbit/oak/OakAssert.java  |   2 +-
 .../org/apache/jackrabbit/oak/api/TreeTest.java    |   6 +-
 .../oak/composite/AtomicCompositeMergeTest.java    |   6 +-
 .../CompositeNodeStoreClusterObservationTest.java  |   2 +-
 .../oak/composite/CompositeNodeStoreTest.java      |   6 +-
 .../jackrabbit/oak/core/MutableTreeTest.java       |   2 +-
 .../org/apache/jackrabbit/oak/core/RootTest.java   |  16 ++
 .../blob/DocumentBlobGCRegistrationTest.java       |   2 +-
 .../blob/DocumentBlobTrackerRegistrationTest.java  |   2 +-
 .../blob/datastore/DataStoreTrackerGCTest.java     |  12 +-
 .../DocumentCachingDataStoreStatsTest.java         |   2 +-
 .../index/AsyncIndexUpdateClusterTestIT.java       |   6 +-
 .../plugins/index/AsyncIndexUpdateLeaseTest.java   |   4 +-
 .../name/ReadWriteNamespaceRegistryTest.java       |   2 +-
 .../oak/segment/SegmentAzureDataStoreBlobGCIT.java |   2 +-
 .../oak/segment/SegmentBlobGCRegistrationTest.java |   2 +-
 .../segment/SegmentCachingDataStoreStatsTest.java  |   2 +-
 .../oak/segment/SegmentS3DataStoreBlobGCIT.java    |   2 +-
 .../oak/spi/commit/CommitContextTest.java          |   2 +-
 .../jackrabbit/oak/spi/state/CheckpointTest.java   |   2 +-
 .../jackrabbit/oak/spi/state/NodeStoreTest.java    |   8 +-
 oak-jackrabbit-api/pom.xml                         |   3 +-
 .../org/apache/jackrabbit/api/JackrabbitNode.java  |   8 +-
 .../jackrabbit/api/JackrabbitNodeTypeManager.java  |   2 +
 .../jackrabbit/api/JackrabbitRepository.java       |   3 +
 .../api/JackrabbitRepositoryFactory.java           |   2 +
 .../apache/jackrabbit/api/JackrabbitSession.java   |   2 +
 .../org/apache/jackrabbit/api/JackrabbitValue.java |   3 +
 .../apache/jackrabbit/api/JackrabbitWorkspace.java |   2 +
 .../org/apache/jackrabbit/api/ReferenceBinary.java |   3 +
 .../jackrabbit/api/ReferenceBinaryException.java   |   3 +
 .../java/org/apache/jackrabbit/api/XASession.java  |   3 +
 .../jackrabbit/api/jmx/EventListenerMBean.java     |   3 +
 .../jackrabbit/api/jmx/ManagedRepositoryMBean.java |   3 +
 .../jackrabbit/api/jmx/QueryStatManagerMBean.java  |   2 +
 .../apache/jackrabbit/api/jmx/package-info.java    |   2 +-
 .../api/management/DataStoreGarbageCollector.java  |   3 +
 .../api/management/MarkEventListener.java          |   3 +
 .../api/management/RepositoryManager.java          |   3 +
 .../jackrabbit/api/management/package-info.java    |   2 +-
 .../api/observation/JackrabbitEvent.java           |   3 +
 .../api/observation/JackrabbitEventFilter.java     |   3 +
 .../observation/JackrabbitObservationManager.java  |   3 +
 .../jackrabbit/api/observation/package-info.java   |   2 +-
 .../org/apache/jackrabbit/api/package-info.java    |   3 +-
 .../api/query/JackrabbitQueryResult.java           |   3 +
 .../apache/jackrabbit/api/query/package-info.java  |   2 +-
 .../authentication/token/TokenCredentials.java     |   9 +-
 .../authentication/token/package-info.java         |   2 +-
 .../security/authorization/PrincipalSetPolicy.java |   2 +
 .../authorization/PrivilegeCollection.java         |  15 +-
 .../security/authorization/PrivilegeManager.java   |   8 +-
 .../api/security/authorization/package-info.java   |   2 +-
 .../api/security/principal/ItemBasedPrincipal.java |   6 +-
 .../security/principal/JackrabbitPrincipal.java    |   3 +
 .../api/security/principal/PrincipalIterator.java  |   7 +-
 .../api/security/principal/PrincipalManager.java   |   6 +-
 .../api/security/principal/package-info.java       |   2 +-
 .../jackrabbit/api/security/user/Authorizable.java |   8 +-
 .../security/user/AuthorizableExistsException.java |   3 +
 .../security/user/AuthorizableTypeException.java   |   3 +
 .../apache/jackrabbit/api/security/user/Group.java |   8 +-
 .../api/security/user/Impersonation.java           |   9 +-
 .../apache/jackrabbit/api/security/user/Query.java |   2 +
 .../jackrabbit/api/security/user/QueryBuilder.java |   6 +-
 .../apache/jackrabbit/api/security/user/User.java  |   8 +-
 .../jackrabbit/api/security/user/package-info.java |   2 +-
 .../org/apache/jackrabbit/api/stats/QueryStat.java |   3 +
 .../apache/jackrabbit/api/stats/QueryStatDto.java  |   3 +
 .../jackrabbit/api/stats/RepositoryStatistics.java |   2 +
 .../apache/jackrabbit/api/stats/TimeSeries.java    |   3 +
 .../apache/jackrabbit/api/stats/package-info.java  |   2 +-
 .../jackrabbit/oak/jcr/TransientMoveTest.java      | 111 +++++++++++
 oak-parent/pom.xml                                 |   4 +-
 .../oak/index/ElasticDocumentStoreIndexer.java     |   2 +-
 .../jackrabbit/oak/index/ElasticIndexCommand.java  |  10 +-
 .../oak/index/ElasticIndexImporterSupport.java     |   2 +-
 .../jackrabbit/oak/run/AvailableElasticModes.java  |   2 +-
 .../org/apache/jackrabbit/oak/index/ReindexIT.java |  17 +-
 .../jackrabbit/oak/run/AzuriteDockerRule.java      | 106 ----------
 .../oak/run/DataStoreCopyCommandTest.java          |   1 +
 oak-search-elastic/pom.xml                         |  10 +
 .../index/elastic/index/ElasticIndexHelper.java    |   3 +
 .../index/elastic/query/ElasticIndexPlanner.java   |   1 +
 .../oak/plugins/index/DynamicBoostCommonTest.java  |  10 +-
 .../oak/plugins/index/OrderByCommonTest.java       |  59 ++++++
 .../oak/plugins/index/PropertyIndexCommonTest.java |  56 ++++--
 .../oak/segment/aws/AwsTarWriterTest.java          |  30 ++-
 oak-segment-azure/pom.xml                          |  14 +-
 .../oak/segment/azure/AzurePersistence.java        |  28 ++-
 .../segment/azure/AzureSegmentArchiveWriter.java   |  30 ++-
 .../oak/segment/azure/tool/SegmentCopy.java        |  13 +-
 .../segment/azure/tool/SegmentStoreMigrator.java   |  74 ++-----
 .../jackrabbit/oak/segment/azure/util/Retrier.java |  82 ++++++++
 .../segment/azure/tool/SegmentCopyTestBase.java    |   3 +-
 .../oak/segment/azure/AzureArchiveManagerTest.java |   1 +
 .../oak/segment/azure/AzureGCJournalTest.java      |   2 +
 .../oak/segment/azure/AzureJournalFileTest.java    |   1 +
 .../oak/segment/azure/AzureManifestFileTest.java   |   2 +
 .../oak/segment/azure/AzureReadSegmentTest.java    |   1 +
 .../oak/segment/azure/AzureRepositoryLockTest.java |   2 +
 .../azure/AzureSegmentArchiveWriterTest.java       | 218 +++++++++++++++++++++
 .../azure/AzureSegmentStoreServiceTest.java        |   2 +
 .../oak/segment/azure/AzureTarFileTest.java        |   2 +
 .../oak/segment/azure/AzureTarFilesTest.java       |   2 +
 .../oak/segment/azure/AzureTarWriterTest.java      |  40 ++--
 .../oak/segment/azure/AzuriteDockerRule.java       | 106 ----------
 .../azure/journal/AzureJournalReaderTest.java      |   3 +-
 .../azure/journal/AzureTarRevisionsTest.java       |   3 +-
 .../azure/journal/ReverseFileReaderTest.java       |   3 +-
 .../oak/segment/azure/tool/ToolUtilsTest.java      |   3 +-
 .../oak/segment/azure/util/RetrierTest.java        | 147 ++++++++++++++
 .../split/SplitPersistenceBlobTest.java            |   2 +-
 .../persistence/split/SplitPersistenceTest.java    |   2 +-
 .../src/test/resources/logback-test.xml            |  11 +-
 .../persistentcache/PersistentRedisCacheTest.java  |   2 +-
 .../jackrabbit/oak/segment/file/FileStore.java     |   3 +
 .../file/UnrecoverableArchiveException.java        |  19 +-
 .../jackrabbit/oak/segment/file/tar/TarWriter.java |  11 +-
 .../jackrabbit/oak/segment/file/FileStoreTest.java | 134 +++++++++++--
 .../oak/segment/file/tar/TarWriterTest.java        |  75 +++++--
 .../src/test/resources/logback-test.xml            |   1 +
 oak-shaded-guava/pom.xml                           |   4 +-
 .../plugins/document/VersionGCRecommendations.java |  83 +++++---
 .../oak/plugins/document/VersionGCSupport.java     |  25 ++-
 .../plugins/document/VersionGarbageCollector.java  |  36 ++--
 .../document/mongo/MongoVersionGCSupport.java      |  45 +++--
 .../oak/plugins/document/rdb/RDBDocumentStore.java |  16 +-
 .../plugins/document/rdb/RDBDocumentStoreJDBC.java |  16 +-
 .../plugins/document/rdb/RDBVersionGCSupport.java  |  41 ++--
 .../oak/plugins/document/util/Utils.java           |  20 +-
 .../oak/plugins/document/VersionGCInitTest.java    |  46 ++++-
 .../oak/plugins/document/VersionGCSupportTest.java |  98 +++++++--
 .../document/VersionGarbageCollectorIT.java        |  52 ++++-
 .../plugins/document/mongo/MongoDockerRule.java    |  41 +++-
 .../oak/plugins/memory/MemoryNodeBuilder.java      |   7 +-
 oak-upgrade/pom.xml                                |  19 +-
 .../jackrabbit/oak/upgrade/AsciiArtTicker.java     |   2 +-
 .../jackrabbit/oak/upgrade/BundleLoader.java       |   2 +-
 .../oak/upgrade/JackrabbitNodeState.java           |  22 +--
 .../jackrabbit/oak/upgrade/PersistingDiff.java     |   6 +-
 .../oak/upgrade/RepositorySidegrade.java           |  12 +-
 .../jackrabbit/oak/upgrade/RepositoryUpgrade.java  |  32 +--
 .../oak/upgrade/SameNameSiblingsEditor.java        |   8 +-
 .../jackrabbit/oak/upgrade/SimpleTicker.java       |   2 +-
 .../oak/upgrade/blob/LengthCachingDataStore.java   |  12 +-
 .../upgrade/checkpoint/CheckpointRetriever.java    |   6 +-
 .../jackrabbit/oak/upgrade/cli/CliUtils.java       |   2 +-
 .../oak/upgrade/cli/MigrationFactory.java          |   4 +-
 .../jackrabbit/oak/upgrade/cli/OakUpgrade.java     |   4 +-
 .../upgrade/cli/blob/AzureDataStoreFactory.java    |   6 +-
 .../oak/upgrade/cli/blob/BlobStoreFactory.java     |   2 +-
 .../upgrade/cli/blob/ConstantBlobStoreFactory.java |   2 +-
 .../upgrade/cli/blob/DummyBlobStoreFactory.java    |   2 +-
 .../oak/upgrade/cli/blob/FileBlobStoreFactory.java |   2 +-
 .../oak/upgrade/cli/blob/FileDataStoreFactory.java |   2 +-
 .../oak/upgrade/cli/blob/LoopbackBlobStore.java    |   2 +-
 .../upgrade/cli/blob/LoopbackBlobStoreFactory.java |   4 +-
 .../oak/upgrade/cli/blob/S3DataStoreFactory.java   |   6 +-
 .../upgrade/cli/blob/SafeDataStoreBlobStore.java   |   2 +-
 .../oak/upgrade/cli/node/Jackrabbit2Factory.java   |   2 +-
 .../oak/upgrade/cli/node/JdbcFactory.java          |   2 +-
 .../oak/upgrade/cli/node/MongoFactory.java         |   2 +-
 .../oak/upgrade/cli/node/NodeStoreFactory.java     |   2 +-
 .../oak/upgrade/cli/node/SegmentAzureFactory.java  |   4 +-
 .../oak/upgrade/cli/node/SegmentFactory.java       |   2 +-
 .../oak/upgrade/cli/node/SegmentTarFactory.java    |   2 +-
 .../oak/upgrade/cli/node/StoreFactory.java         |   2 +-
 .../oak/upgrade/cli/parser/DatastoreArguments.java |   2 +-
 .../upgrade/nodestate/NameFilteringNodeState.java  |   2 +-
 .../oak/upgrade/CopyCheckpointsTest.java           |   2 +-
 .../oak/upgrade/CopyVersionHistoryTest.java        |   4 +-
 .../oak/upgrade/IgnoreMissingBinariesTest.java     |   2 +-
 .../oak/upgrade/PrivilegeUpgradeTest.java          |   8 +-
 .../oak/upgrade/RepositoryUpgradeTest.java         |   2 +-
 .../oak/upgrade/SameNodeSiblingsTest.java          |   4 +-
 .../oak/upgrade/UpgradeOldSegmentTest.java         |   2 +-
 .../upgrade/blob/LengthCachingDataStoreTest.java   |   4 +-
 .../oak/upgrade/cli/AbstractOak2OakTest.java       |   6 +-
 .../upgrade/cli/SegmentAzureToSegmentTarTest.java  |   2 +-
 .../upgrade/cli/SegmentTarToSegmentAzureTest.java  |   2 +-
 .../oak/upgrade/cli/blob/CopyBinariesTest.java     |   2 +-
 .../cli/blob/LoopbackBlobStoreFactoryTest.java     |   2 +-
 .../cli/container/AzureDataStoreContainer.java     |   2 +-
 .../cli/container/FileDataStoreContainer.java      |   2 +-
 .../cli/container/JdbcNodeStoreContainer.java      |   2 +-
 .../cli/container/MongoNodeStoreContainer.java     |   2 +-
 .../cli/container/S3DataStoreContainer.java        |   2 +-
 .../container/SegmentAzureNodeStoreContainer.java  |   4 +-
 .../oak/upgrade/util/VersionCopyTestUtils.java     |   2 +-
 272 files changed, 2443 insertions(+), 1478 deletions(-)
 create mode 100644 oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipCycleTest.java
 copy oak-benchmarks/src/main/java/org/apache/jackrabbit/oak/benchmark/{StringWriteTest.java => AccessAfterMoveTest.java} (57%)
 delete mode 100644 oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/GuavaDeprecation.java
 copy oak-it/src/test/java/org/apache/jackrabbit/oak/core/RootTest.java => oak-core/src/test/java/org/apache/jackrabbit/oak/core/MoveTest.java (64%)
 create mode 100644 oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TransientMoveTest.java
 delete mode 100644 oak-run/src/test/java/org/apache/jackrabbit/oak/run/AzuriteDockerRule.java
 create mode 100644 oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/util/Retrier.java
 create mode 100644 oak-segment-azure/src/test/java/org/apache/jackrabbit/oak/segment/azure/AzureSegmentArchiveWriterTest.java
 delete mode 100644 oak-segment-azure/src/test/java/org/apache/jackrabbit/oak/segment/azure/AzuriteDockerRule.java
 create mode 100644 oak-segment-azure/src/test/java/org/apache/jackrabbit/oak/segment/azure/util/RetrierTest.java
 copy {oak-segment-tar => oak-segment-azure}/src/test/resources/logback-test.xml (71%)
 copy oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/util/Environment.java => oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/UnrecoverableArchiveException.java (67%)


[jackrabbit-oak] 08/08: OAK-10199 : updated logic to fetch nodes by sorting them on the basis of _modified & _id

Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

daim pushed a commit to branch OAK-10199
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit 580cd8e4149853a72466b1bd1f85a4415802e3b9
Author: Rishabh Kumar <di...@adobe.com>
AuthorDate: Mon Jun 19 13:45:20 2023 +0530

    OAK-10199 : updated logic to fetch nodes by sorting them on the basis of _modified & _id
---
 .../plugins/document/VersionGCRecommendations.java | 83 +++++++++++-------
 .../oak/plugins/document/VersionGCSupport.java     | 25 +++---
 .../plugins/document/VersionGarbageCollector.java  | 36 +++++---
 .../document/mongo/MongoVersionGCSupport.java      | 45 +++++-----
 .../oak/plugins/document/rdb/RDBDocumentStore.java | 16 +++-
 .../plugins/document/rdb/RDBDocumentStoreJDBC.java | 16 ++--
 .../plugins/document/rdb/RDBVersionGCSupport.java  | 41 ++++-----
 .../oak/plugins/document/util/Utils.java           | 20 +++--
 .../oak/plugins/document/VersionGCInitTest.java    | 46 ++++++++--
 .../oak/plugins/document/VersionGCSupportTest.java | 98 ++++++++++++++++++----
 .../document/VersionGarbageCollectorIT.java        | 52 +++++++++++-
 11 files changed, 342 insertions(+), 136 deletions(-)

diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
index 4584d925c0..056c2fe438 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
@@ -20,11 +20,11 @@ package org.apache.jackrabbit.oak.plugins.document;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats;
 import org.apache.jackrabbit.oak.plugins.document.util.TimeInterval;
-import org.apache.jackrabbit.oak.plugins.document.util.Utils;
 import org.apache.jackrabbit.oak.spi.gc.GCMonitor;
 import org.apache.jackrabbit.oak.stats.Clock;
 import org.slf4j.Logger;
@@ -32,8 +32,14 @@ import org.slf4j.LoggerFactory;
 
 import static java.lang.Long.MAX_VALUE;
 import static java.util.Map.of;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MIN_ID_VALUE;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.NULL;
+import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP;
 import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP;
+import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_ID;
 import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP;
+import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.timestampToString;
 
 /**
  * Gives a recommendation about parameters for the next revision garbage collection run.
@@ -47,13 +53,13 @@ public class VersionGCRecommendations {
 
     final boolean ignoreDueToCheckPoint;
     final TimeInterval scope;
-    final TimeInterval scopeFullGC;
+    final TimeInterval scopeDetailedGC;
     final long maxCollect;
     final long deleteCandidateCount;
     final long lastOldestTimestamp;
     final long detailedGCTimestamp;
+    final String detailedGCId;
     final long originalCollectLimit;
-
     private final long precisionMs;
     final long suggestedIntervalMs;
     private final boolean scopeIsComplete;
@@ -86,7 +92,8 @@ public class VersionGCRecommendations {
         long deletedOnceCount = 0;
         long suggestedIntervalMs;
         long oldestPossible;
-        long oldestPossibleFullGC;
+        long oldestModifiedDocTimeStamp;
+        String oldestModifiedDocId;
         long collectLimit = options.collectLimit;
 
         this.vgc = vgc;
@@ -95,12 +102,12 @@ public class VersionGCRecommendations {
 
         TimeInterval keep = new TimeInterval(clock.getTime() - maxRevisionAgeMs, Long.MAX_VALUE);
 
-        Map<String, Long> settings = getLongSettings();
-        lastOldestTimestamp = settings.get(VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP);
+        Map<String, Object> settings = getVGCSettings();
+        lastOldestTimestamp = (long) settings.get(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP);
         if (lastOldestTimestamp == 0) {
-            log.debug("No lastOldestTimestamp found, querying for the oldest deletedOnce candidate");
+            log.info("No lastOldestTimestamp found, querying for the oldest deletedOnce candidate");
             oldestPossible = vgc.getOldestDeletedOnceTimestamp(clock, options.precisionMs) - 1;
-            log.debug("lastOldestTimestamp found: {}", Utils.timestampToString(oldestPossible));
+            log.info("lastOldestTimestamp found: {}", timestampToString(oldestPossible));
         } else {
             oldestPossible = lastOldestTimestamp - 1;
         }
@@ -108,23 +115,27 @@ public class VersionGCRecommendations {
         TimeInterval scope = new TimeInterval(oldestPossible, Long.MAX_VALUE);
         scope = scope.notLaterThan(keep.fromMs);
 
-        detailedGCTimestamp = settings.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP);
-        if (detailedGCTimestamp == 0) {
-            if (log.isDebugEnabled()) {
-                log.debug("No detailedGCTimestamp found, querying for the oldest deletedOnce candidate");
-            }
-            oldestPossibleFullGC = vgc.getOldestModifiedTimestamp(clock) - 1;
-            if (log.isDebugEnabled()) {
-                log.debug("detailedGCTimestamp found: {}", Utils.timestampToString(oldestPossibleFullGC));
+        detailedGCTimestamp = (long) settings.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP);
+        oldestModifiedDocId = (String) settings.get(SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP);
+        if (detailedGCTimestamp == 0 || Objects.equals(oldestModifiedDocId, MIN_ID_VALUE)) {
+            log.info("No detailedGCTimestamp found, querying for the oldest modified candidate");
+            final NodeDocument doc = vgc.getOldestModifiedDoc(clock);
+            if (doc == NULL) {
+                oldestModifiedDocTimeStamp = 0L;
+                oldestModifiedDocId = MIN_ID_VALUE;
+            } else {
+                oldestModifiedDocId = doc.getId();
+                oldestModifiedDocTimeStamp = doc.getModified() == null ? 0L : doc.getModified() - 1;
             }
+            log.info("detailedGCTimestamp found: {}", timestampToString(oldestModifiedDocTimeStamp));
         } else {
-            oldestPossibleFullGC = detailedGCTimestamp - 1;
+            oldestModifiedDocTimeStamp = detailedGCTimestamp - 1;
         }
 
-        TimeInterval fullGCTimeInternal = new TimeInterval(oldestPossibleFullGC, MAX_VALUE);
-        fullGCTimeInternal = fullGCTimeInternal.notLaterThan(keep.fromMs);
+        TimeInterval detailedGCTimeInternal = new TimeInterval(oldestModifiedDocTimeStamp, MAX_VALUE);
+        detailedGCTimeInternal = detailedGCTimeInternal.notLaterThan(keep.fromMs);
 
-        suggestedIntervalMs = settings.get(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP);
+        suggestedIntervalMs = (long) settings.get(SETTINGS_COLLECTION_REC_INTERVAL_PROP);
         if (suggestedIntervalMs > 0) {
             suggestedIntervalMs = Math.max(suggestedIntervalMs, options.precisionMs);
             if (suggestedIntervalMs < scope.getDurationMs()) {
@@ -168,7 +179,7 @@ public class VersionGCRecommendations {
                 ignoreDueToCheckPoint = true;
             } else {
                 scope = scope.notLaterThan(checkpoint.getTimestamp() - 1);
-                log.debug("checkpoint at [{}] found, scope now {}", Utils.timestampToString(checkpoint.getTimestamp()), scope);
+                log.debug("checkpoint at [{}] found, scope now {}", timestampToString(checkpoint.getTimestamp()), scope);
             }
         }
 
@@ -182,7 +193,8 @@ public class VersionGCRecommendations {
         this.precisionMs = options.precisionMs;
         this.ignoreDueToCheckPoint = ignoreDueToCheckPoint;
         this.scope = scope;
-        this.scopeFullGC = fullGCTimeInternal;
+        this.scopeDetailedGC = detailedGCTimeInternal;
+        this.detailedGCId = oldestModifiedDocId;
         this.scopeIsComplete = scope.toMs >= keep.fromMs;
         this.maxCollect = collectLimit;
         this.suggestedIntervalMs = suggestedIntervalMs;
@@ -207,7 +219,8 @@ public class VersionGCRecommendations {
         } else if (!stats.canceled && !stats.ignoredGCDueToCheckPoint) {
             // success, we would not expect to encounter revisions older than this in the future
             setLongSetting(of(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs,
-                    SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP, stats.oldestModifiedGced));
+                    SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp));
+            setStringSetting(SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP, stats.oldestModifiedDocId);
 
             int count = stats.deletedDocGCCount - stats.deletedLeafDocGCCount;
             double usedFraction;
@@ -224,7 +237,7 @@ public class VersionGCRecommendations {
                     long nextDuration = (long) Math.ceil(suggestedIntervalMs * 1.5);
                     log.debug("successful run using {}% of limit, raising recommended interval to {} seconds",
                             Math.round(usedFraction * 1000) / 10.0, TimeUnit.MILLISECONDS.toSeconds(nextDuration));
-                    setLongSetting(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, nextDuration);
+                    setLongSetting(SETTINGS_COLLECTION_REC_INTERVAL_PROP, nextDuration);
                 } else {
                     log.debug("not increasing limit: collected {} documents ({}% >= {}% limit)", count, usedFraction,
                             allowedFraction);
@@ -236,19 +249,23 @@ public class VersionGCRecommendations {
         }
     }
 
-    private Map<String, Long> getLongSettings() {
-        Document versionGCDoc = vgc.getDocumentStore().find(Collection.SETTINGS, VersionGarbageCollector.SETTINGS_COLLECTION_ID, 0);
-        Map<String, Long> settings = new HashMap<>();
+    private Map<String, Object> getVGCSettings() {
+        Document versionGCDoc = vgc.getDocumentStore().find(Collection.SETTINGS, SETTINGS_COLLECTION_ID, 0);
+        Map<String, Object> settings = new HashMap<>();
         // default values
-        settings.put(VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, 0L);
-        settings.put(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, 0L);
+        settings.put(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, 0L);
+        settings.put(SETTINGS_COLLECTION_REC_INTERVAL_PROP, 0L);
         settings.put(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP, 0L);
+        settings.put(SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP, MIN_ID_VALUE);
         if (versionGCDoc != null) {
             for (String k : versionGCDoc.keySet()) {
                 Object value = versionGCDoc.get(k);
                 if (value instanceof Number) {
                     settings.put(k, ((Number) value).longValue());
                 }
+                if (value instanceof String) {
+                    settings.put(k, value);
+                }
             }
         }
         return settings;
@@ -258,8 +275,14 @@ public class VersionGCRecommendations {
         setLongSetting(of(propName, val));
     }
 
+    private void setStringSetting(String propName, String val) {
+        UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true);
+        updateOp.set(propName, val);
+        vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
+    }
+
     private void setLongSetting(final Map<String, Long> propValMap) {
-        UpdateOp updateOp = new UpdateOp(VersionGarbageCollector.SETTINGS_COLLECTION_ID, true);
+        UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true);
         propValMap.forEach(updateOp::set);
         vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
     }
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java
index f23340acbc..abdfdf4a64 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java
@@ -19,9 +19,12 @@
 
 package org.apache.jackrabbit.oak.plugins.document;
 
+import static java.util.Comparator.comparing;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
 import static java.util.stream.Collectors.toList;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MIN_ID_VALUE;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_IN_SECS;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.NULL;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.getModifiedInSecs;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getAllDocuments;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getSelectedDocuments;
@@ -52,7 +55,7 @@ public class VersionGCSupport {
 
     /**
      * Returns documents that have a {@link NodeDocument#MODIFIED_IN_SECS} value
-     * within the given range and the {@link NodeDocument#DELETED} set to
+     * within the given range and the {@link NodeDocument#  DELETED} set to
      * {@code true}. The two passed modified timestamps are in milliseconds
      * since the epoch and the implementation will convert them to seconds at
      * the granularity of the {@link NodeDocument#MODIFIED_IN_SECS} field and
@@ -79,12 +82,15 @@ public class VersionGCSupport {
      * @param fromModified the lower bound modified timestamp (inclusive)
      * @param toModified the upper bound modified timestamp (exclusive)
      * @param limit the limit of documents to return
+     * @param fromId the lower bound {@link NodeDocument#ID} (exclusive)
      * @return matching documents.
      */
-    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified, final int limit) {
+    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified, final int limit,
+                                                  final String fromId) {
         return StreamSupport
-                .stream(getSelectedDocuments(store, MODIFIED_IN_SECS, fromModified).spliterator(), false)
+                .stream(getSelectedDocuments(store, MODIFIED_IN_SECS, 1, fromId).spliterator(), false)
                 .filter(input -> modifiedGreaterThanEquals(input, fromModified) && modifiedLessThan(input, toModified))
+                .sorted((o1, o2) -> comparing(NodeDocument::getModified).thenComparing(Document::getId).compare(o1, o2))
                 .limit(limit)
                 .collect(toList());
     }
@@ -176,27 +182,26 @@ public class VersionGCSupport {
     }
 
     /**
-     * Retrieve the time of the oldest modified document.
+     * Retrieve the oldest modified document.
      *
-     * @return the timestamp of the oldest modified document.
+     * @return the oldest modified document.
      */
-    public long getOldestModifiedTimestamp(final Clock clock) {
+    public NodeDocument getOldestModifiedDoc(final Clock clock) {
         long ts = 0;
         long now = clock.getTime();
         Iterable<NodeDocument> docs = null;
 
         LOG.info("find oldest modified document");
         try {
-            docs = getModifiedDocs(ts, now, 1);
+            docs = getModifiedDocs(ts, now, 1, MIN_ID_VALUE);
             if (docs.iterator().hasNext()) {
-                Long modified = docs.iterator().next().getModified();
-                return modified != null ? modified : 0L;
+                return docs.iterator().next();
             }
         } finally {
             Utils.closeIfCloseable(docs);
         }
         LOG.info("find oldest modified document to be {}", Utils.timestampToString(ts));
-        return ts;
+        return NULL;
     }
 
     public long getDeletedOnceCount() throws UnsupportedOperationException {
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
index b562831e24..f54299e3fd 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
@@ -32,7 +32,6 @@ import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.stream.Collectors;
 
 import org.apache.jackrabbit.guava.common.base.Function;
 import org.apache.jackrabbit.guava.common.base.Joiner;
@@ -112,6 +111,11 @@ public class VersionGarbageCollector {
      */
     static final String SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP = "detailedGCTimeStamp";
 
+    /**
+     * Property name to _id till when last detailed-GC run happened
+     */
+    static final String SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP = "detailedGCId";
+
     private final DocumentNodeStore nodeStore;
     private final DocumentStore ds;
     private final boolean detailedGCEnabled;
@@ -272,7 +276,8 @@ public class VersionGarbageCollector {
         int splitDocGCCount;
         int intermediateSplitDocGCCount;
         int updateResurrectedGCCount;
-        long oldestModifiedGced;
+        long oldestModifiedDocTimeStamp;
+        String oldestModifiedDocId;
         int updatedDetailedGCDocsCount;
         int deletedPropsGCCount;
         final TimeDurationFormatter df = TimeDurationFormatter.forLogging();
@@ -343,7 +348,8 @@ public class VersionGarbageCollector {
                     ", updateResurrectedGCCount=" + updateResurrectedGCCount +
                     ", splitDocGCCount=" + splitDocGCCount +
                     ", intermediateSplitDocGCCount=" + intermediateSplitDocGCCount +
-                    ", oldestModifiedGced=" + oldestModifiedGced +
+                    ", oldestModifiedDocId=" + oldestModifiedDocId +
+                    ", oldestModifiedDocTimeStamp=" + oldestModifiedDocTimeStamp +
                     ", updatedDetailedGCDocsCount=" + updatedDetailedGCDocsCount +
                     ", deletedPropsGCCount=" + deletedPropsGCCount +
                     ", iterationCount=" + iterationCount +
@@ -363,7 +369,8 @@ public class VersionGarbageCollector {
             this.splitDocGCCount += run.splitDocGCCount;
             this.intermediateSplitDocGCCount += run.intermediateSplitDocGCCount;
             this.updateResurrectedGCCount += run.updateResurrectedGCCount;
-            this.oldestModifiedGced = run.oldestModifiedGced;
+            this.oldestModifiedDocTimeStamp = run.oldestModifiedDocTimeStamp;
+            this.oldestModifiedDocId = run.oldestModifiedDocId;
             this.updatedDetailedGCDocsCount += run.updatedDetailedGCDocsCount;
             this.deletedPropsGCCount += run.deletedPropsGCCount;
             if (run.iterationCount > 0) {
@@ -608,15 +615,16 @@ public class VersionGarbageCollector {
                 throws IOException {
             int docsTraversed = 0;
             boolean foundDoc = true;
-            long oldestModifiedGCed = rec.scopeFullGC.fromMs;
+            long oldestModifiedDocTimeStamp = rec.scopeDetailedGC.fromMs;
+            String oldestModifiedDocId = rec.detailedGCId;
             try (DetailedGC gc = new DetailedGC(headRevision, monitor, cancel)) {
-                final long fromModified = rec.scopeFullGC.fromMs;
-                final long toModified = rec.scopeFullGC.toMs;
+                final long fromModified = rec.scopeDetailedGC.fromMs;
+                final long toModified = rec.scopeDetailedGC.toMs;
                 if (phases.start(GCPhase.DETAILED_GC)) {
-                    while (foundDoc && oldestModifiedGCed < toModified && docsTraversed <= PROGRESS_BATCH_SIZE) {
+                    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(oldestModifiedGCed, toModified, 1000);
+                        Iterable<NodeDocument> itr = versionStore.getModifiedDocs(oldestModifiedDocTimeStamp, toModified, 1000, oldestModifiedDocId);
                         try {
                             for (NodeDocument doc : itr) {
                                 foundDoc = true;
@@ -640,12 +648,12 @@ public class VersionGarbageCollector {
                                 if (modified == null) {
                                     monitor.warn("collectDetailGarbage : document has no _modified property : {}",
                                             doc.getId());
-                                } else if (modified < oldestModifiedGCed) {
+                                } else if (modified < oldestModifiedDocTimeStamp) {
                                     monitor.warn(
                                             "collectDetailGarbage : document has older _modified than query boundary : {} (from: {}, to: {})",
                                             modified, fromModified, toModified);
                                 } else {
-                                    oldestModifiedGCed = modified;
+                                    oldestModifiedDocTimeStamp = modified;
                                 }
 
                                 if (gc.hasGarbage() && phases.start(GCPhase.DETAILED_GC_CLEANUP)) {
@@ -653,11 +661,13 @@ public class VersionGarbageCollector {
                                     phases.stop(GCPhase.DETAILED_GC_CLEANUP);
                                 }
 
-                                oldestModifiedGCed = modified == null ? fromModified : modified;
+                                oldestModifiedDocTimeStamp = modified == null ? fromModified : modified;
+                                oldestModifiedDocId = doc.getId();
                             }
                         } finally {
                             Utils.closeIfCloseable(itr);
-                            phases.stats.oldestModifiedGced = oldestModifiedGCed;
+                            phases.stats.oldestModifiedDocTimeStamp = oldestModifiedDocTimeStamp;
+                            phases.stats.oldestModifiedDocId = oldestModifiedDocId;
                         }
                     }
                     phases.stop(GCPhase.DETAILED_GC);
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
index 324ade704c..690fd5a0d6 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
@@ -21,8 +21,8 @@ package org.apache.jackrabbit.oak.plugins.document.mongo;
 
 import static com.mongodb.client.model.Filters.eq;
 import static com.mongodb.client.model.Filters.exists;
+import static com.mongodb.client.model.Filters.gt;
 import static java.util.Optional.ofNullable;
-import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.concat;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.transform;
@@ -34,6 +34,7 @@ import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
 import static org.apache.jackrabbit.oak.plugins.document.Document.ID;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.DELETED_ONCE;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_IN_SECS;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.NULL;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.PATH;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SD_MAX_REV_TIME_IN_SECS;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SD_TYPE;
@@ -130,21 +131,26 @@ public class MongoVersionGCSupport extends VersionGCSupport {
 
     /**
      * Returns documents that have a {@link NodeDocument#MODIFIED_IN_SECS} value
-     * within the given range in sorted order. The two passed modified timestamps
-     * are in milliseconds since the epoch and the implementation will convert them
-     * to seconds at the granularity of the {@link NodeDocument#MODIFIED_IN_SECS}
-     * field and then perform the comparison.
+     * within the given range .The two passed modified timestamps are in milliseconds
+     * since the epoch and the implementation will convert them to seconds at
+     * the granularity of the {@link NodeDocument#MODIFIED_IN_SECS} field and
+     * then perform the comparison.
      *
      * @param fromModified the lower bound modified timestamp (inclusive)
-     * @param toModified   the upper bound modified timestamp (exclusive)
-     * @return matching documents in sorted order of {@link NodeDocument#MODIFIED_IN_SECS}
+     * @param toModified the upper bound modified timestamp (exclusive)
+     * @param limit the limit of documents to return
+     * @param fromId the lower bound {@link NodeDocument#ID} (exclusive)
+     * @return matching documents.
      */
     @Override
-    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified, final int limit) {
-        // _modified >= fromModified && _modified < toModified
+    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified, final int limit,
+                                                  final String fromId) {
+        // _modified >= fromModified && _modified < toModified && _id > fromId
         final Bson query = and(gte(MODIFIED_IN_SECS, getModifiedInSecs(fromModified)),
-                lt(MODIFIED_IN_SECS, getModifiedInSecs(toModified)));
-        final Bson sort = eq(MODIFIED_IN_SECS, 1);
+                lt(MODIFIED_IN_SECS, getModifiedInSecs(toModified)), gt(ID, fromId));
+        // first sort by _modified and then by _id
+        final Bson sort = and(eq(MODIFIED_IN_SECS, 1), eq(ID, 1));
+
         final FindIterable<BasicDBObject> cursor = getNodeCollection()
                 .find(query)
                 .sort(sort)
@@ -232,11 +238,11 @@ public class MongoVersionGCSupport extends VersionGCSupport {
      * @return the timestamp of the oldest modified document.
      */
     @Override
-    public long getOldestModifiedTimestamp(final Clock clock) {
-        LOG.info("getOldestModifiedTimestamp() <- start");
+    public NodeDocument getOldestModifiedDoc(final Clock clock) {
+        LOG.info("getOldestModifiedDoc() <- start");
 
-        final Bson sort = eq(MODIFIED_IN_SECS, 1);
-        final List<Long> result = new ArrayList<>(1);
+        final Bson sort = and(eq(MODIFIED_IN_SECS, 1), eq(ID, 1));
+        final List<NodeDocument> result = new ArrayList<>(1);
 
         // we need to add query condition to ignore `previous` documents which doesn't have this field
         final Bson query = exists(MODIFIED_IN_SECS);
@@ -245,14 +251,13 @@ public class MongoVersionGCSupport extends VersionGCSupport {
                 (Consumer<BasicDBObject>) document ->
                         ofNullable(store.convertFromDBObject(NODES, document))
                                 .ifPresent(doc -> {
-                    long modifiedMs = SECONDS.toMillis(ofNullable(doc.getModified()).orElse(0L));
-                    LOG.info("getOldestDeletedOnceTimestamp() -> {}", Utils.timestampToString(modifiedMs));
-                    result.add(modifiedMs);
+                    LOG.info("getOldestModifiedDoc() -> {}", doc);
+                    result.add(doc);
                 }));
 
         if (result.isEmpty()) {
-            LOG.info("getOldestModifiedTimestamp() -> none found, return current time");
-            result.add(clock.getTime());
+            LOG.info("getOldestModifiedDoc() -> none found, return NULL document");
+            result.add(NULL);
         }
         return result.get(0);
     }
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java
index 82c09e213d..3a9ef2d95d 100755
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java
@@ -971,8 +971,8 @@ public class RDBDocumentStore implements DocumentStore {
     public static String VERSIONPROP = "__version";
 
     // set of supported indexed properties
-    private static final Set<String> INDEXEDPROPERTIES = new HashSet<String>(Arrays.asList(new String[] { MODIFIED,
-            NodeDocument.HAS_BINARY_FLAG, NodeDocument.DELETED_ONCE, NodeDocument.SD_TYPE, NodeDocument.SD_MAX_REV_TIME_IN_SECS, VERSIONPROP }));
+    private static final Set<String> INDEXEDPROPERTIES = new HashSet<>(Arrays.asList(MODIFIED,
+            NodeDocument.HAS_BINARY_FLAG, NodeDocument.DELETED_ONCE, NodeDocument.SD_TYPE, NodeDocument.SD_MAX_REV_TIME_IN_SECS, VERSIONPROP, ID));
 
     // set of required table columns
     private static final Set<String> REQUIREDCOLUMNS = Collections.unmodifiableSet(new HashSet<String>(Arrays.asList(
@@ -1840,7 +1840,7 @@ public class RDBDocumentStore implements DocumentStore {
     }
 
     protected <T extends Document> Iterable<T> queryAsIterable(final Collection<T> collection, String fromKey, String toKey,
-            final List<String> excludeKeyPatterns, final List<QueryCondition> conditions, final int limit, final String sortBy) {
+            final List<String> excludeKeyPatterns, final List<QueryCondition> conditions, final int limit, final List<String> sortBy) {
 
         final RDBTableMetaData tmd = getTable(collection);
         Set<String> allowedProps = Sets.intersection(INDEXEDPROPERTIES, tmd.getColumnProperties());
@@ -1853,6 +1853,16 @@ public class RDBDocumentStore implements DocumentStore {
             }
         }
 
+        if (sortBy != null && !sortBy.isEmpty()) {
+            for (String key: sortBy) {
+                if (!allowedProps.contains(key)) {
+                    final String message = "indexed property " + key + " not supported. supported properties are " + allowedProps;
+                    LOG.error(message);
+                    throw new UnsupportedIndexedPropertyException(message);
+                }
+            }
+        }
+
         final String from = collection == Collection.NODES && NodeDocument.MIN_ID_VALUE.equals(fromKey) ? null : fromKey;
         final String to = collection == Collection.NODES && NodeDocument.MAX_ID_VALUE.equals(toKey) ? null : toKey;
 
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStoreJDBC.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStoreJDBC.java
index 26fc1311fa..59dfb968b0 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStoreJDBC.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStoreJDBC.java
@@ -16,6 +16,8 @@
  */
 package org.apache.jackrabbit.oak.plugins.document.rdb;
 
+import static java.util.List.of;
+import static java.util.stream.Collectors.joining;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.transform;
 import static org.apache.jackrabbit.guava.common.collect.Sets.newHashSet;
 import static org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.CHAR2OCTETRATIO;
@@ -459,7 +461,7 @@ public class RDBDocumentStoreJDBC {
                             + excludeKeyPatterns + ", conditions=" + conditions + ", limit=" + limit)
                     : null);
             stmt = prepareQuery(connection, tmd, fields, minId,
-                    maxId, excludeKeyPatterns, conditions, limit, "ID");
+                    maxId, excludeKeyPatterns, conditions, limit, of("ID"));
             rs = stmt.executeQuery();
             while (rs.next() && result.size() < limit) {
                 int field = 1;
@@ -554,7 +556,7 @@ public class RDBDocumentStoreJDBC {
 
     @NotNull
     public Iterator<RDBRow> queryAsIterator(RDBConnectionHandler ch, RDBTableMetaData tmd, String minId, String maxId,
-            List<String> excludeKeyPatterns, List<QueryCondition> conditions, int limit, String sortBy) throws SQLException {
+            List<String> excludeKeyPatterns, List<QueryCondition> conditions, int limit, List<String> sortBy) throws SQLException {
         return new ResultSetIterator(ch, tmd, minId, maxId, excludeKeyPatterns, conditions, limit, sortBy);
     }
 
@@ -573,7 +575,7 @@ public class RDBDocumentStoreJDBC {
         private long pstart;
 
         public ResultSetIterator(RDBConnectionHandler ch, RDBTableMetaData tmd, String minId, String maxId,
-                List<String> excludeKeyPatterns, List<QueryCondition> conditions, int limit, String sortBy) throws SQLException {
+                List<String> excludeKeyPatterns, List<QueryCondition> conditions, int limit, List<String> sortBy) throws SQLException {
             long start = System.currentTimeMillis();
             try {
                 this.ch = ch;
@@ -695,7 +697,7 @@ public class RDBDocumentStoreJDBC {
 
     @NotNull
     private PreparedStatement prepareQuery(Connection connection, RDBTableMetaData tmd, String columns, String minId, String maxId,
-            List<String> excludeKeyPatterns, List<QueryCondition> conditions, int limit, String sortBy) throws SQLException {
+            List<String> excludeKeyPatterns, List<QueryCondition> conditions, int limit, List<String> sortBy) throws SQLException {
 
         StringBuilder selectClause = new StringBuilder();
 
@@ -714,9 +716,8 @@ public class RDBDocumentStoreJDBC {
             query.append(" where ").append(whereClause);
         }
 
-        if (sortBy != null) {
-            // FIXME : order should be determined via sortBy field
-            query.append(" order by ID");
+        if (sortBy != null && !sortBy.isEmpty()) {
+            query.append(" order by ").append(sortBy.stream().map(INDEXED_PROP_MAPPING::get).collect(joining(", ")));
         }
 
         if (limit != Integer.MAX_VALUE) {
@@ -967,6 +968,7 @@ public class RDBDocumentStoreJDBC {
         tmp.put(NodeDocument.SD_TYPE, "SDTYPE");
         tmp.put(NodeDocument.SD_MAX_REV_TIME_IN_SECS, "SDMAXREVTIME");
         tmp.put(RDBDocumentStore.VERSIONPROP, "VERSION");
+        tmp.put(NodeDocument.ID, "ID");
         INDEXED_PROP_MAPPING = Collections.unmodifiableMap(tmp);
     }
 
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java
index f26268bcd3..0d2f678911 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java
@@ -16,11 +16,13 @@
  */
 package org.apache.jackrabbit.oak.plugins.document.rdb;
 
-import static java.util.Collections.emptyList;
 import static java.util.List.of;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
+import static org.apache.jackrabbit.oak.plugins.document.Document.ID;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MIN_ID_VALUE;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_IN_SECS;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.NULL;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.getModifiedInSecs;
 import static org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.EMPTY_KEY_PATTERN;
 
@@ -99,18 +101,21 @@ public class RDBVersionGCSupport extends VersionGCSupport {
      * then perform the comparison.
      *
      * @param fromModified the lower bound modified timestamp (inclusive)
-     * @param toModified   the upper bound modified timestamp (exclusive)
-     * @param limit        the limit of documents to return
+     * @param toModified the upper bound modified timestamp (exclusive)
+     * @param limit the limit of documents to return
+     * @param fromId the lower bound {@link NodeDocument#ID} (exclusive)
      * @return matching documents.
      */
     @Override
-    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified, final int limit) {
+    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified, final int limit,
+                                                  final String fromId) {
         List<QueryCondition> conditions = of(new QueryCondition(MODIFIED_IN_SECS, "<", getModifiedInSecs(toModified)),
-                new QueryCondition(MODIFIED_IN_SECS, ">=", getModifiedInSecs(fromModified)));
+                new QueryCondition(MODIFIED_IN_SECS, ">=", getModifiedInSecs(fromModified)),
+                new QueryCondition(ID, ">", of(fromId)));
         if (MODE == 1) {
             return getIterator(EMPTY_KEY_PATTERN, conditions);
         } else {
-            return store.queryAsIterable(NODES, null, null, EMPTY_KEY_PATTERN, conditions, limit, MODIFIED_IN_SECS);
+            return store.queryAsIterable(NODES, fromId, null, EMPTY_KEY_PATTERN, conditions, limit, of(MODIFIED_IN_SECS, ID));
         }
     }
 
@@ -275,24 +280,20 @@ public class RDBVersionGCSupport extends VersionGCSupport {
      * @return the timestamp of the oldest modified document.
      */
     @Override
-    public long getOldestModifiedTimestamp(Clock clock) {
-        long modifiedMs = Long.MIN_VALUE;
+    public NodeDocument getOldestModifiedDoc(Clock clock) {
+        NodeDocument doc = NULL;
 
-        LOG.info("getOldestModifiedTimestamp() <- start");
+        LOG.info("getOldestModifiedDoc() <- start");
+        Iterable<NodeDocument> modifiedDocs = null;
         try {
-            long modifiedSec = store.getMinValue(NODES, MODIFIED_IN_SECS, null, null, EMPTY_KEY_PATTERN, emptyList());
-            modifiedMs = TimeUnit.SECONDS.toMillis(modifiedSec);
+            modifiedDocs = getModifiedDocs(0L, clock.getTime(), 1, MIN_ID_VALUE);
+            doc = modifiedDocs.iterator().hasNext() ? modifiedDocs.iterator().next() : NULL;
         } catch (DocumentStoreException ex) {
-            LOG.error("getOldestModifiedTimestamp()", ex);
-        }
-
-        if (modifiedMs > 0) {
-            LOG.info("getOldestModifiedTimestamp() -> {}", Utils.timestampToString(modifiedMs));
-            return modifiedMs;
-        } else {
-            LOG.info("getOldestModifiedTimestamp() -> none found, return current time");
-            return clock.getTime();
+            LOG.error("getOldestModifiedDoc()", ex);
+        } finally {
+            Utils.closeIfCloseable(modifiedDocs);
         }
+        return doc;
     }
 
     @Override
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
index c9428429bc..85bb1225fb 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
@@ -62,6 +62,7 @@ import org.slf4j.LoggerFactory;
 
 import static org.apache.jackrabbit.guava.common.base.Preconditions.checkNotNull;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.transform;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MIN_ID_VALUE;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.isDeletedEntry;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.isCommitRootEntry;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.isRevisionsEntry;
@@ -668,7 +669,7 @@ public class Utils {
      * @return an {@link Iterable} over all documents in the store.
      */
     public static Iterable<NodeDocument> getAllDocuments(final DocumentStore store) {
-        return internalGetSelectedDocuments(store, null, 0, DEFAULT_BATCH_SIZE);
+        return internalGetSelectedDocuments(store, null, 0, MIN_ID_VALUE, DEFAULT_BATCH_SIZE);
     }
 
     /**
@@ -710,7 +711,7 @@ public class Utils {
      */
     public static Iterable<NodeDocument> getSelectedDocuments(
             DocumentStore store, String indexedProperty, long startValue, int batchSize) {
-        return internalGetSelectedDocuments(store, indexedProperty, startValue, batchSize);
+        return internalGetSelectedDocuments(store, indexedProperty, startValue, MIN_ID_VALUE, batchSize);
     }
 
     /**
@@ -719,12 +720,21 @@ public class Utils {
      */
     public static Iterable<NodeDocument> getSelectedDocuments(
             DocumentStore store, String indexedProperty, long startValue) {
-        return internalGetSelectedDocuments(store, indexedProperty, startValue, DEFAULT_BATCH_SIZE);
+        return internalGetSelectedDocuments(store, indexedProperty, startValue, MIN_ID_VALUE, DEFAULT_BATCH_SIZE);
+    }
+
+    /**
+     * Like {@link #getSelectedDocuments(DocumentStore, String, long, int)} with
+     * a default {@code batchSize}.
+     */
+    public static Iterable<NodeDocument> getSelectedDocuments(
+            DocumentStore store, String indexedProperty, long startValue, String fromId) {
+        return internalGetSelectedDocuments(store, indexedProperty, startValue, fromId, DEFAULT_BATCH_SIZE);
     }
 
     private static Iterable<NodeDocument> internalGetSelectedDocuments(
             final DocumentStore store, final String indexedProperty,
-            final long startValue, final int batchSize) {
+            final long startValue, String fromId, final int batchSize) {
         if (batchSize < 2) {
             throw new IllegalArgumentException("batchSize must be > 1");
         }
@@ -733,7 +743,7 @@ public class Utils {
             public Iterator<NodeDocument> iterator() {
                 return new AbstractIterator<NodeDocument>() {
 
-                    private String startId = NodeDocument.MIN_ID_VALUE;
+                    private String startId = fromId;
 
                     private Iterator<NodeDocument> batch = nextBatch();
 
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java
index ed39a372b2..6aceeac830 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java
@@ -22,14 +22,19 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 
+import static java.util.concurrent.TimeUnit.DAYS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
+import static org.apache.jackrabbit.oak.plugins.document.Collection.SETTINGS;
 import static org.apache.jackrabbit.oak.plugins.document.DetailGCHelper.enableDetailGC;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MIN_ID_VALUE;
+import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP;
 import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getIdFromPath;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
-import java.util.concurrent.TimeUnit;
-
 public class VersionGCInitTest {
 
     @Rule
@@ -45,27 +50,50 @@ public class VersionGCInitTest {
     @Test
     public void lazyInitialize() throws Exception {
         DocumentStore store = ns.getDocumentStore();
-        Document vgc = store.find(Collection.SETTINGS, "versionGC");
+        Document vgc = store.find(SETTINGS, "versionGC");
         assertNull(vgc);
 
-        ns.getVersionGarbageCollector().gc(1, TimeUnit.DAYS);
+        ns.getVersionGarbageCollector().gc(1, DAYS);
 
-        vgc = store.find(Collection.SETTINGS, "versionGC");
+        vgc = store.find(SETTINGS, "versionGC");
         assertNotNull(vgc);
         assertEquals(0L, vgc.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP));
+        assertNull(vgc.get(SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP));
     }
 
     @Test
     public void lazyInitializeWithDetailedGC() throws Exception {
         DocumentStore store = ns.getDocumentStore();
-        Document vgc = store.find(Collection.SETTINGS, "versionGC");
+        Document vgc = store.find(SETTINGS, "versionGC");
         assertNull(vgc);
 
         enableDetailGC(ns.getVersionGarbageCollector());
-        ns.getVersionGarbageCollector().gc(1, TimeUnit.DAYS);
+        long offset = SECONDS.toMillis(42);
+        String id = getIdFromPath("/node");
+        Revision r = new Revision(offset, 0, 1);
+        UpdateOp op = new UpdateOp(id, true);
+        NodeDocument.setModified(op, r);
+        store.createOrUpdate(NODES, op);
+        ns.getVersionGarbageCollector().gc(1, DAYS);
 
-        vgc = store.find(Collection.SETTINGS, "versionGC");
+        vgc = store.find(SETTINGS, "versionGC");
         assertNotNull(vgc);
-        assertEquals(-1L, vgc.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP));
+        assertEquals(39L, vgc.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP));
+        assertEquals(id, vgc.get(SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP));
+    }
+
+    @Test
+    public void lazyInitializeWithDetailedGCWithNoData() throws Exception {
+        DocumentStore store = ns.getDocumentStore();
+        Document vgc = store.find(SETTINGS, "versionGC");
+        assertNull(vgc);
+
+        enableDetailGC(ns.getVersionGarbageCollector());
+        ns.getVersionGarbageCollector().gc(1, DAYS);
+
+        vgc = store.find(SETTINGS, "versionGC");
+        assertNotNull(vgc);
+        assertEquals(0L, vgc.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP));
+        assertEquals(MIN_ID_VALUE, vgc.get(SETTINGS_COLLECTION_DETAILED_GC_DOCUMENT_ID_PROP));
     }
 }
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupportTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupportTest.java
index 831d2c89ec..4eb20986c2 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupportTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupportTest.java
@@ -18,10 +18,9 @@
  */
 package org.apache.jackrabbit.oak.plugins.document;
 
+import java.util.ArrayList;
 import java.util.List;
 
-import org.apache.jackrabbit.guava.common.collect.Iterables;
-import org.apache.jackrabbit.guava.common.collect.Lists;
 import com.mongodb.ReadPreference;
 
 import org.apache.jackrabbit.oak.plugins.document.mongo.MongoDocumentStore;
@@ -29,31 +28,38 @@ import org.apache.jackrabbit.oak.plugins.document.mongo.MongoTestUtils;
 import org.apache.jackrabbit.oak.plugins.document.mongo.MongoVersionGCSupport;
 import org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore;
 import org.apache.jackrabbit.oak.plugins.document.rdb.RDBVersionGCSupport;
-import org.apache.jackrabbit.oak.plugins.document.util.Utils;
-import org.apache.jackrabbit.oak.stats.Clock;
 import org.junit.After;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
+import static java.util.Comparator.comparing;
+import static java.util.List.of;
+import static java.util.Optional.ofNullable;
 import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.stream.StreamSupport.stream;
+import static org.apache.jackrabbit.guava.common.collect.Comparators.isInOrder;
+import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentStoreFixture.MEMORY;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentStoreFixture.MONGO;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentStoreFixture.RDB_H2;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MIN_ID_VALUE;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getIdFromPath;
+import static org.apache.jackrabbit.oak.stats.Clock.SIMPLE;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 @RunWith(Parameterized.class)
 public class VersionGCSupportTest {
 
-    private DocumentStoreFixture fixture;
-    private DocumentStore store;
-    private VersionGCSupport gcSupport;
-    private List<String> ids = Lists.newArrayList();
+    private final DocumentStoreFixture fixture;
+    private final DocumentStore store;
+    private final VersionGCSupport gcSupport;
+    private final List<String> ids = new ArrayList<>();
 
     @Parameterized.Parameters(name="{0}")
     public static java.util.Collection<DocumentStoreFixture> fixtures() {
-        List<DocumentStoreFixture> fixtures = Lists.newArrayList();
+        List<DocumentStoreFixture> fixtures = new ArrayList<>(3);
         if (RDB_H2.isAvailable()) {
             fixtures.add(RDB_H2);
         }
@@ -88,7 +94,7 @@ public class VersionGCSupportTest {
 
     @After
     public void after() throws Exception {
-        store.remove(Collection.NODES, ids);
+        store.remove(NODES, ids);
         fixture.dispose();
     }
 
@@ -97,12 +103,12 @@ public class VersionGCSupportTest {
         long offset = SECONDS.toMillis(42);
         for (int i = 0; i < 5; i++) {
             Revision r = new Revision(offset + SECONDS.toMillis(i), 0, 1);
-            String id = Utils.getIdFromPath("/doc-" + i);
+            String id = getIdFromPath("/doc-" + i);
             ids.add(id);
             UpdateOp op = new UpdateOp(id, true);
             NodeDocument.setModified(op, r);
             NodeDocument.setDeleted(op, r, true);
-            store.create(Collection.NODES, Lists.newArrayList(op));
+            store.create(NODES, of(op));
         }
 
         assertPossiblyDeleted(0, 41, 0);
@@ -126,25 +132,83 @@ public class VersionGCSupportTest {
         assertPossiblyDeleted(51, 60, 0);
     }
 
+    @Test
+    public void getPossiblyModifiedDocs() {
+        long offset = SECONDS.toMillis(42);
+        for (int i = 0; i < 5; i++) {
+            Revision r = new Revision(offset + SECONDS.toMillis(i), 0, 1);
+            String id = getIdFromPath("/doc-modified" + i);
+            ids.add(id);
+            UpdateOp op = new UpdateOp(id, true);
+            NodeDocument.setModified(op, r);
+            store.create(NODES, of(op));
+        }
+
+        assertModified(0, 41, 0);
+        assertModified(0, 42, 0);
+        assertModified(0, 44, 0);
+        assertModified(0, 45, 3);
+        assertModified(0, 46, 3);
+        assertModified(0, 49, 3);
+        assertModified(0, 50, 5);
+        assertModified(0, 51, 5);
+        assertModified(39, 60, 5);
+        assertModified(40, 60, 5);
+        assertModified(41, 60, 5);
+        assertModified(42, 60, 5);
+        assertModified(44, 60, 5);
+        assertModified(45, 60, 2);
+        assertModified(47, 60, 2);
+        assertModified(48, 60, 2);
+        assertModified(49, 60, 2);
+        assertModified(50, 60, 0);
+        assertModified(51, 60, 0);
+    }
+
     @Test
     public void findOldest() {
         // see OAK-8476
         long secs = 123456;
         long offset = SECONDS.toMillis(secs);
         Revision r = new Revision(offset, 0, 1);
-        String id = Utils.getIdFromPath("/doc-del");
+        String id = getIdFromPath("/doc-del");
         ids.add(id);
         UpdateOp op = new UpdateOp(id, true);
         NodeDocument.setModified(op, r);
         NodeDocument.setDeleted(op, r, true);
-        store.create(Collection.NODES, Lists.newArrayList(op));
+        store.create(NODES, of(op));
 
-        long reportedsecs = gcSupport.getOldestDeletedOnceTimestamp(Clock.SIMPLE, 1) / SECONDS.toMillis(1);
-        assertTrue("diff (s) should be < 5: " + Math.abs(secs - reportedsecs), Math.abs(secs - reportedsecs) < 5);
+        long reportedSecs = gcSupport.getOldestDeletedOnceTimestamp(SIMPLE, 1) / SECONDS.toMillis(1);
+        assertTrue("diff (s) should be < 5: " + Math.abs(secs - reportedSecs), Math.abs(secs - reportedSecs) < 5);
+    }
+
+    @Test
+    public void findOldestModified() {
+        long secs = 1234567;
+        long offset = SECONDS.toMillis(secs);
+        Revision r = new Revision(offset, 0, 1);
+        String id = getIdFromPath("/doc-modified");
+        ids.add(id);
+        UpdateOp op = new UpdateOp(id, true);
+        NodeDocument.setModified(op, r);
+        store.create(NODES, of(op));
+
+        NodeDocument oldestModifiedDoc = gcSupport.getOldestModifiedDoc(SIMPLE);
+        String oldestModifiedDocId = oldestModifiedDoc.getId();
+        long reportedSecs = ofNullable(oldestModifiedDoc.getModified()).orElse(0L);
+        assertTrue("diff (s) should be < 5: " + Math.abs(secs - reportedSecs), Math.abs(secs - reportedSecs) < 5);
+        assertEquals(id, oldestModifiedDocId);
     }
 
     private void assertPossiblyDeleted(long fromSeconds, long toSeconds, long num) {
         Iterable<NodeDocument> docs = gcSupport.getPossiblyDeletedDocs(SECONDS.toMillis(fromSeconds), SECONDS.toMillis(toSeconds));
-        assertEquals(num, Iterables.size(docs));
+        assertEquals(num, stream(docs.spliterator(), false).count());
+    }
+
+    private void assertModified(long fromSeconds, long toSeconds, long num) {
+        Iterable<NodeDocument> docs = gcSupport.getModifiedDocs(SECONDS.toMillis(fromSeconds), SECONDS.toMillis(toSeconds), 10, MIN_ID_VALUE);
+        docs.forEach(d -> System.out.println(d.getModified() + " " + d.getId()));
+        assertEquals(num, stream(docs.spliterator(), false).count());
+        assertTrue(isInOrder(docs, (o1, o2) -> comparing(NodeDocument::getModified).thenComparing(Document::getId).compare(o1, o2)));
     }
 }
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
index 4470a07f96..caa156a6d4 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
@@ -229,8 +229,8 @@ public class VersionGarbageCollectorIT {
     }
 
     @Test
-    public void testGCDeletedProps() throws Exception{
-        //1. Create nodes
+    public void testGCDeletedProps() throws Exception {
+        //1. Create nodes with properties
         NodeBuilder b1 = store.getRoot().builder();
 
         // Add property to node & save
@@ -289,6 +289,54 @@ public class VersionGarbageCollectorIT {
         stats = gc.gc(maxAge*2, HOURS);
         assertEquals(0, stats.deletedPropsGCCount);
     }
+
+    // Test when we have more than 1000 deleted properties
+    @Test
+    public void testGCDeletedProps_1() throws Exception {
+        //1. Create nodes with properties
+        NodeBuilder b1 = store.getRoot().builder();
+
+        // Add property to node & save
+        for (int i = 0; i < 5_000; i++) {
+            for (int j = 0; j < 10; j++) {
+                b1.child("z"+i).setProperty("prop"+j, "foo", STRING);
+            }
+        }
+        store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        // enable the detailed gc flag
+        writeField(gc, "detailedGCEnabled", true, true);
+        long maxAge = 1; //hours
+        long delta = TimeUnit.MINUTES.toMillis(10);
+        //1. Go past GC age and check no GC done as nothing deleted
+        clock.waitUntil(Revision.getCurrentTimestamp() + maxAge);
+        VersionGCStats stats = gc.gc(maxAge, HOURS);
+        assertEquals(0, stats.deletedPropsGCCount);
+
+        //Remove property
+        NodeBuilder b2 = store.getRoot().builder();
+        for (int i = 0; i < 5_000; i++) {
+            for (int j = 0; j < 10; j++) {
+                b2.getChildNode("z"+i).removeProperty("prop"+j);
+            }
+        }
+        store.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        store.runBackgroundOperations();
+
+        //2. Check that a deleted property is not collected before maxAge
+        //Clock cannot move back (it moved forward in #1) so double the maxAge
+        clock.waitUntil(clock.getTime() + delta);
+        stats = gc.gc(maxAge*2, HOURS);
+        assertEquals(0, stats.deletedPropsGCCount);
+
+        //3. Check that deleted property does get collected post maxAge
+        clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
+
+        stats = gc.gc(maxAge*2, HOURS);
+        assertEquals(50_000, stats.deletedPropsGCCount);
+
+    }
     
     private void gcSplitDocsInternal(String subNodeName) throws Exception {
         long maxAge = 1; //hrs


[jackrabbit-oak] 02/08: OAK-10199 : disable the detailGc in tearDown to avoid side-effects

Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

daim pushed a commit to branch OAK-10199
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit fdee87a36c3d9121f6cc0ef209b5bd378feea3fc
Author: Stefan Egli <st...@apache.org>
AuthorDate: Thu Apr 20 18:38:45 2023 +0200

    OAK-10199 : disable the detailGc in tearDown to avoid side-effects
---
 .../java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java   | 1 +
 1 file changed, 1 insertion(+)

diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
index 1bd81ce89c..445e7c4275 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
@@ -95,6 +95,7 @@ public class VersionGCTest {
 
     @After
     public void tearDown() throws Exception {
+        DetailGCHelper.disableDetailGC(ns);
         execService.shutdown();
         execService.awaitTermination(1, MINUTES);
     }


[jackrabbit-oak] 03/08: OAK-10199 : provided support for feature toggle & osgi config for detailed gc

Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

daim pushed a commit to branch OAK-10199
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit f0dcb48535bbf49de79b772ad02dd7740bab62ac
Author: Rishabh Kumar <di...@adobe.com>
AuthorDate: Mon Apr 24 13:07:44 2023 +0530

    OAK-10199 : provided support for feature toggle & osgi config for detailed gc
---
 .../plugins/document/DocumentNodeStoreHelper.java  |   4 +-
 .../jackrabbit/oak/run/RevisionsCommand.java       |   3 +-
 .../oak/plugins/document/Configuration.java        |   1 +
 .../oak/plugins/document/DocumentNodeStore.java    |   3 +-
 .../plugins/document/DocumentNodeStoreBuilder.java |   1 +
 .../plugins/document/DocumentNodeStoreService.java |   8 +
 .../oak/plugins/document/NodeDocument.java         |  16 ++
 .../plugins/document/VersionGCRecommendations.java |  43 ++++-
 .../oak/plugins/document/VersionGCSupport.java     |  98 ++++++-----
 .../plugins/document/VersionGarbageCollector.java  | 183 ++++++++++++---------
 .../document/mongo/MongoVersionGCSupport.java      |  42 ++++-
 .../oak/plugins/document/util/Utils.java           |  11 ++
 .../DocumentNodeStoreServiceConfigurationTest.java |   1 +
 .../oak/plugins/document/VersionGCQueryTest.java   |   4 +-
 .../oak/plugins/document/VersionGCTest.java        |  11 +-
 .../document/VersionGarbageCollectorIT.java        |   8 +-
 .../mongo/MongoDocumentNodeStoreBuilderTest.java   |  12 ++
 .../oak/plugins/document/util/UtilsTest.java       |  41 ++++-
 18 files changed, 345 insertions(+), 145 deletions(-)

diff --git a/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreHelper.java b/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreHelper.java
index cf839c88c5..3b39d4c3a1 100644
--- a/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreHelper.java
+++ b/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreHelper.java
@@ -72,8 +72,8 @@ public class DocumentNodeStoreHelper {
     }
 
     public static VersionGarbageCollector createVersionGC(
-            DocumentNodeStore nodeStore, VersionGCSupport gcSupport) {
-        return new VersionGarbageCollector(nodeStore, gcSupport);
+            DocumentNodeStore nodeStore, VersionGCSupport gcSupport, final boolean detailedGCEnabled) {
+        return new VersionGarbageCollector(nodeStore, gcSupport, detailedGCEnabled);
     }
 
     private static Iterable<BlobReferences> scan(DocumentNodeStore store,
diff --git a/oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java b/oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java
index e418148952..b995910c57 100644
--- a/oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java
+++ b/oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java
@@ -60,6 +60,7 @@ import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreHelper.createVersionGC;
 import static org.apache.jackrabbit.oak.plugins.document.FormatVersion.versionOf;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getRootDocument;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.isDetailedGCEnabled;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.timestampToString;
 import static org.apache.jackrabbit.oak.run.Utils.asCloseable;
 import static org.apache.jackrabbit.oak.run.Utils.createDocumentMKBuilder;
@@ -226,7 +227,7 @@ public class RevisionsCommand implements Command {
         useMemoryBlobStore(builder);
         // create a version GC that operates on a read-only DocumentNodeStore
         // and a GC support with a writable DocumentStore
-        VersionGarbageCollector gc = createVersionGC(builder.build(), gcSupport);
+        VersionGarbageCollector gc = createVersionGC(builder.build(), gcSupport, isDetailedGCEnabled(builder));
 
         VersionGCOptions gcOptions = gc.getOptions();
         gcOptions = gcOptions.withDelayFactor(options.getDelay());
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java
index d9a00cb28d..ae7aa143d2 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java
@@ -32,6 +32,7 @@ import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreBuilde
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreBuilder.DEFAULT_NODE_CACHE_PERCENTAGE;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreBuilder.DEFAULT_PREV_DOC_CACHE_PERCENTAGE;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreBuilder.DEFAULT_UPDATE_LIMIT;
+import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.DEFAULT_DETAILED_GC_ENABLED;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.DEFAULT_THROTTLING_ENABLED;
 
 @ObjectClassDefinition(
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
index 22b6244792..8d876fca40 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
@@ -38,6 +38,7 @@ import static org.apache.jackrabbit.oak.plugins.document.Path.ROOT;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.alignWithExternalRevisions;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getIdFromPath;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getModuleVersion;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.isDetailedGCEnabled;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.isThrottlingEnabled;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.pathToId;
 import static org.apache.jackrabbit.oak.spi.observation.ChangeSet.COMMIT_CONTEXT_OBSERVATION_CHANGESET;
@@ -641,7 +642,7 @@ public final class DocumentNodeStore
         this.branches = new UnmergedBranches();
         this.asyncDelay = builder.getAsyncDelay();
         this.versionGarbageCollector = new VersionGarbageCollector(
-                this, builder.createVersionGCSupport());
+                this, builder.createVersionGCSupport(), isDetailedGCEnabled(builder));
         this.versionGarbageCollector.setStatisticsProvider(builder.getStatisticsProvider());
         this.versionGarbageCollector.setGCMonitor(builder.getGCMonitor());
         this.journalGarbageCollector = new JournalGarbageCollector(
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
index 6d93278b5d..aa3ab1ea81 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
@@ -125,6 +125,7 @@ public class DocumentNodeStoreBuilder<T extends DocumentNodeStoreBuilder<T>> {
     private boolean isReadOnlyMode = false;
     private Feature prefetchFeature;
     private Feature docStoreThrottlingFeature;
+    private Feature docStoreDetailedGCFeature;
     private Weigher<CacheValue, CacheValue> weigher = new EmpiricalWeigher();
     private long memoryCacheSize = DEFAULT_MEMORY_CACHE_SIZE;
     private int nodeCachePercentage = DEFAULT_NODE_CACHE_PERCENTAGE;
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
index 42bf88c120..ad01cda3ca 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
@@ -138,6 +138,7 @@ public class DocumentNodeStoreService {
     static final String DEFAULT_DB = "oak";
     static final boolean DEFAULT_SO_KEEP_ALIVE = true;
     static final boolean DEFAULT_THROTTLING_ENABLED = false;
+    static final boolean DEFAULT_DETAILED_GC_ENABLED = false;
     static final int DEFAULT_MONGO_LEASE_SO_TIMEOUT_MILLIS = 30000;
     static final String DEFAULT_PERSISTENT_CACHE = "cache";
     static final String DEFAULT_JOURNAL_CACHE = "diff-cache";
@@ -181,6 +182,11 @@ public class DocumentNodeStoreService {
      */
     private static final String FT_NAME_DOC_STORE_THROTTLING = "FT_THROTTLING_OAK-9909";
 
+    /**
+     * Feature toggle name to enable detailed GC for Mongo Document Store
+     */
+    private static final String FT_NAME_DEATILED_GC = "FT_DETAILED_GC_OAK-10199";
+
     // property name constants - values can come from framework properties or OSGi config
     public static final String CUSTOM_BLOB_STORE = "customBlobStore";
     public static final String PROP_REV_RECOVERY_INTERVAL = "lastRevRecoveryJobIntervalInSecs";
@@ -216,6 +222,7 @@ public class DocumentNodeStoreService {
     private JournalPropertyHandlerFactory journalPropertyHandlerFactory = new JournalPropertyHandlerFactory();
     private Feature prefetchFeature;
     private Feature docStoreThrottlingFeature;
+    private Feature docStoreDetailedGCFeature;
     private ComponentContext context;
     private Whiteboard whiteboard;
     private long deactivationTimestamp = 0;
@@ -250,6 +257,7 @@ public class DocumentNodeStoreService {
         documentStoreType = DocumentStoreType.fromString(this.config.documentStoreType());
         prefetchFeature = Feature.newFeature(FT_NAME_PREFETCH, whiteboard);
         docStoreThrottlingFeature = Feature.newFeature(FT_NAME_DOC_STORE_THROTTLING, whiteboard);
+        docStoreDetailedGCFeature = Feature.newFeature(FT_NAME_DEATILED_GC, whiteboard);
 
         registerNodeStoreIfPossible();
     }
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
index c79c6adfc4..71abba0a2e 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
@@ -32,6 +32,7 @@ import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
+import java.util.stream.Collectors;
 
 import org.apache.jackrabbit.guava.common.base.Function;
 import org.apache.jackrabbit.guava.common.base.Predicate;
@@ -66,6 +67,7 @@ import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.mergeSorted;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.transform;
 import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toMap;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
 import static org.apache.jackrabbit.oak.plugins.document.StableRevisionComparator.REVERSE;
 import static org.apache.jackrabbit.oak.plugins.document.UpdateOp.Key;
@@ -1669,6 +1671,20 @@ public final class NodeDocument extends Document {
         return map;
     }
 
+    /**
+     * Returns all the properties on this document
+     * @return Map of all properties along with their values
+     */
+    @NotNull
+    Map<String, SortedMap<Revision, String>> getProperties() {
+        return data
+                .keySet()
+                .stream()
+                .filter(Utils::isPropertyName)
+                .map(o -> Map.entry(o, getLocalMap(o)))
+                .collect(toMap(Entry::getKey, Entry::getValue));
+    }
+
     /**
      * @return the {@link #REVISIONS} stored on this document.
      */
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
index ac47cc69d8..d8b091261d 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
@@ -21,6 +21,7 @@ package org.apache.jackrabbit.oak.plugins.document;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
+import com.google.common.collect.ImmutableMap;
 import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats;
 import org.apache.jackrabbit.oak.plugins.document.util.TimeInterval;
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
@@ -31,6 +32,9 @@ import org.slf4j.LoggerFactory;
 
 import org.apache.jackrabbit.guava.common.collect.Maps;
 
+import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP;
+import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP;
+
 /**
  * Gives a recommendation about parameters for the next revision garbage collection run.
  */
@@ -43,6 +47,7 @@ public class VersionGCRecommendations {
 
     final boolean ignoreDueToCheckPoint;
     final TimeInterval scope;
+    final TimeInterval scopeFullGC;
     final long maxCollect;
     final long deleteCandidateCount;
     final long lastOldestTimestamp;
@@ -81,6 +86,7 @@ public class VersionGCRecommendations {
         long deletedOnceCount = 0;
         long suggestedIntervalMs;
         long oldestPossible;
+        long oldestPossibleFullGC;
         long collectLimit = options.collectLimit;
 
         this.vgc = vgc;
@@ -90,7 +96,7 @@ public class VersionGCRecommendations {
         TimeInterval keep = new TimeInterval(clock.getTime() - maxRevisionAgeMs, Long.MAX_VALUE);
 
         Map<String, Long> settings = getLongSettings();
-        lastOldestTimestamp = settings.get(VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP);
+        lastOldestTimestamp = settings.get(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP);
         if (lastOldestTimestamp == 0) {
             log.debug("No lastOldestTimestamp found, querying for the oldest deletedOnce candidate");
             oldestPossible = vgc.getOldestDeletedOnceTimestamp(clock, options.precisionMs) - 1;
@@ -102,7 +108,21 @@ public class VersionGCRecommendations {
         TimeInterval scope = new TimeInterval(oldestPossible, Long.MAX_VALUE);
         scope = scope.notLaterThan(keep.fromMs);
 
-        fullDetailGCTimestamp = settings.get(VersionGarbageCollector.SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP);
+        fullDetailGCTimestamp = settings.get(SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP);
+        if (fullDetailGCTimestamp == 0) {
+            if (log.isDebugEnabled()) {
+                log.debug("No fullDetailGCTimestamp found, querying for the oldest deletedOnce candidate");
+            }
+            oldestPossibleFullGC = vgc.getOldestModifiedTimestamp(clock) - 1;
+            if (log.isDebugEnabled()) {
+                log.debug("fullDetailGCTimestamp found: {}", Utils.timestampToString(oldestPossibleFullGC));
+            }
+        } else {
+            oldestPossibleFullGC = fullDetailGCTimestamp - 1;
+        }
+
+        TimeInterval scopeFullGC = new TimeInterval(oldestPossibleFullGC, Long.MAX_VALUE);
+        scopeFullGC = scopeFullGC.notLaterThan(keep.fromMs);
 
         suggestedIntervalMs = settings.get(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP);
         if (suggestedIntervalMs > 0) {
@@ -162,6 +182,7 @@ public class VersionGCRecommendations {
         this.precisionMs = options.precisionMs;
         this.ignoreDueToCheckPoint = ignoreDueToCheckPoint;
         this.scope = scope;
+        this.scopeFullGC = scopeFullGC;
         this.scopeIsComplete = scope.toMs >= keep.fromMs;
         this.maxCollect = collectLimit;
         this.suggestedIntervalMs = suggestedIntervalMs;
@@ -185,7 +206,10 @@ public class VersionGCRecommendations {
             stats.needRepeat = true;
         } else if (!stats.canceled && !stats.ignoredGCDueToCheckPoint) {
             // success, we would not expect to encounter revisions older than this in the future
-            setLongSetting(VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs);
+//            setLongSetting(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs);
+//            setLongSetting(SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, stats.oldestModifiedGced);
+            setLongSetting(ImmutableMap.of(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs,
+                    SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, stats.oldestModifiedGced));
 
             int count = stats.deletedDocGCCount - stats.deletedLeafDocGCCount;
             double usedFraction;
@@ -218,9 +242,9 @@ public class VersionGCRecommendations {
         Document versionGCDoc = vgc.getDocumentStore().find(Collection.SETTINGS, VersionGarbageCollector.SETTINGS_COLLECTION_ID, 0);
         Map<String, Long> settings = Maps.newHashMap();
         // default values
-        settings.put(VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, 0L);
+        settings.put(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, 0L);
         settings.put(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, 0L);
-        settings.put(VersionGarbageCollector.SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, -1L);
+        settings.put(SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, 0L);
         if (versionGCDoc != null) {
             for (String k : versionGCDoc.keySet()) {
                 Object value = versionGCDoc.get(k);
@@ -233,8 +257,15 @@ public class VersionGCRecommendations {
     }
 
     void setLongSetting(String propName, long val) {
+        setLongSetting(Map.of(propName, val));
+//        UpdateOp updateOp = new UpdateOp(VersionGarbageCollector.SETTINGS_COLLECTION_ID, true);
+//        updateOp.set(propName, val);
+//        vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
+    }
+
+    void setLongSetting(final Map<String, Long> propValMap) {
         UpdateOp updateOp = new UpdateOp(VersionGarbageCollector.SETTINGS_COLLECTION_ID, true);
-        updateOp.set(propName, val);
+        propValMap.forEach(updateOp::set);
         vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
     }
 }
\ No newline at end of file
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java
index 0e5c26c83d..f23340acbc 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java
@@ -20,11 +20,14 @@
 package org.apache.jackrabbit.oak.plugins.document;
 
 import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
+import static java.util.stream.Collectors.toList;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_IN_SECS;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.getModifiedInSecs;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getAllDocuments;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getSelectedDocuments;
 
 import java.util.Set;
+import java.util.stream.StreamSupport;
 
 import org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType;
 import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats;
@@ -59,53 +62,40 @@ public class VersionGCSupport {
      * @param toModified the upper bound modified timestamp (exclusive)
      * @return matching documents.
      */
-    public Iterable<NodeDocument> getPossiblyDeletedDocs(final long fromModified,
-                                                         final long toModified) {
-        return filter(getSelectedDocuments(store, NodeDocument.DELETED_ONCE, 1), new Predicate<NodeDocument>() {
-            @Override
-            public boolean apply(NodeDocument input) {
-                return input.wasDeletedOnce()
-                        && modifiedGreaterThanEquals(input, fromModified)
-                        && modifiedLessThan(input, toModified);
-            }
-
-            private boolean modifiedGreaterThanEquals(NodeDocument doc,
-                                                      long time) {
-                Long modified = doc.getModified();
-                return modified != null && modified.compareTo(getModifiedInSecs(time)) >= 0;
-            }
-
-            private boolean modifiedLessThan(NodeDocument doc,
-                                             long time) {
-                Long modified = doc.getModified();
-                return modified != null && modified.compareTo(getModifiedInSecs(time)) < 0;
-            }
-        });
+    public Iterable<NodeDocument> getPossiblyDeletedDocs(final long fromModified, final long toModified) {
+        return StreamSupport
+                .stream(getSelectedDocuments(store, NodeDocument.DELETED_ONCE, 1).spliterator(), false)
+                .filter(input -> input.wasDeletedOnce() && modifiedGreaterThanEquals(input, fromModified) && modifiedLessThan(input, toModified))
+                .collect(toList());
     }
 
     /**
-     * TODO: document me!
+     * Returns documents that have a {@link NodeDocument#MODIFIED_IN_SECS} value
+     * within the given range .The two passed modified timestamps are in milliseconds
+     * since the epoch and the implementation will convert them to seconds at
+     * the granularity of the {@link NodeDocument#MODIFIED_IN_SECS} field and
+     * then perform the comparison.
+     *
+     * @param fromModified the lower bound modified timestamp (inclusive)
+     * @param toModified the upper bound modified timestamp (exclusive)
+     * @param limit the limit of documents to return
+     * @return matching documents.
      */
-    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified) {
-        return filter(getSelectedDocuments(store, NodeDocument.MODIFIED_IN_SECS, fromModified), new Predicate<NodeDocument>() {
-            @Override
-            public boolean apply(NodeDocument input) {
-                return modifiedGreaterThanEquals(input, fromModified)
-                        && modifiedLessThan(input, toModified);
-            }
-
-            private boolean modifiedGreaterThanEquals(NodeDocument doc,
-                                                      long time) {
-                Long modified = doc.getModified();
-                return modified != null && modified.compareTo(getModifiedInSecs(time)) >= 0;
-            }
+    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified, final int limit) {
+        return StreamSupport
+                .stream(getSelectedDocuments(store, MODIFIED_IN_SECS, fromModified).spliterator(), false)
+                .filter(input -> modifiedGreaterThanEquals(input, fromModified) && modifiedLessThan(input, toModified))
+                .limit(limit)
+                .collect(toList());
+    }
 
-            private boolean modifiedLessThan(NodeDocument doc,
-                                             long time) {
-                Long modified = doc.getModified();
-                return modified != null && modified.compareTo(getModifiedInSecs(time)) < 0;
-            }
-        });
+    private boolean modifiedGreaterThanEquals(final NodeDocument doc, final long time) {
+        Long modified = doc.getModified();
+        return modified != null && modified.compareTo(getModifiedInSecs(time)) >= 0;
+    }
+    private boolean modifiedLessThan(final NodeDocument doc, final long time) {
+        Long modified = doc.getModified();
+        return modified != null && modified.compareTo(getModifiedInSecs(time)) < 0;
     }
 
     /**
@@ -185,6 +175,30 @@ public class VersionGCSupport {
         return ts;
     }
 
+    /**
+     * Retrieve the time of the oldest modified document.
+     *
+     * @return the timestamp of the oldest modified document.
+     */
+    public long getOldestModifiedTimestamp(final Clock clock) {
+        long ts = 0;
+        long now = clock.getTime();
+        Iterable<NodeDocument> docs = null;
+
+        LOG.info("find oldest modified document");
+        try {
+            docs = getModifiedDocs(ts, now, 1);
+            if (docs.iterator().hasNext()) {
+                Long modified = docs.iterator().next().getModified();
+                return modified != null ? modified : 0L;
+            }
+        } finally {
+            Utils.closeIfCloseable(docs);
+        }
+        LOG.info("find oldest modified document to be {}", Utils.timestampToString(ts));
+        return ts;
+    }
+
     public long getDeletedOnceCount() throws UnsupportedOperationException {
         throw new UnsupportedOperationException("getDeletedOnceCount()");
     }
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
index e7442a7d15..608ba02398 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
@@ -27,6 +27,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
@@ -54,7 +55,7 @@ import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.apache.jackrabbit.guava.common.base.Preconditions.checkNotNull;
+import static java.util.Objects.requireNonNull;
 import static org.apache.jackrabbit.guava.common.base.StandardSystemProperty.LINE_SEPARATOR;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.all;
 import static org.apache.jackrabbit.guava.common.collect.Iterators.partition;
@@ -66,6 +67,7 @@ import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_I
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType.COMMIT_ROOT_ONLY;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType.DEFAULT_LEAF;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType.DEFAULT_NO_BRANCH;
+import static org.apache.jackrabbit.oak.plugins.document.StableRevisionComparator.REVERSE;
 import static org.slf4j.helpers.MessageFormatter.arrayFormat;
 
 public class VersionGarbageCollector {
@@ -115,6 +117,7 @@ public class VersionGarbageCollector {
 
     private final DocumentNodeStore nodeStore;
     private final DocumentStore ds;
+    private final boolean detailedGCEnabled;
     private final VersionGCSupport versionStore;
     private final AtomicReference<GCJob> collector = newReference();
     private VersionGCOptions options;
@@ -122,10 +125,12 @@ public class VersionGarbageCollector {
     private RevisionGCStats gcStats = new RevisionGCStats(StatisticsProvider.NOOP);
 
     VersionGarbageCollector(DocumentNodeStore nodeStore,
-                            VersionGCSupport gcSupport) {
+                            VersionGCSupport gcSupport,
+                            final boolean detailedGCEnabled) {
         this.nodeStore = nodeStore;
         this.versionStore = gcSupport;
         this.ds = gcSupport.getDocumentStore();
+        this.detailedGCEnabled = detailedGCEnabled;
         this.options = new VersionGCOptions();
     }
 
@@ -201,7 +206,7 @@ public class VersionGarbageCollector {
     }
 
     public void setGCMonitor(@NotNull GCMonitor gcMonitor) {
-        this.gcMonitor = checkNotNull(gcMonitor);
+        this.gcMonitor = requireNonNull(gcMonitor);
     }
 
     public VersionGCOptions getOptions() {
@@ -259,6 +264,7 @@ public class VersionGarbageCollector {
     }
 
     public static class VersionGCStats {
+        public long oldestModifiedGced;
         boolean ignoredGCDueToCheckPoint;
         boolean canceled;
         boolean success = true;
@@ -274,14 +280,14 @@ public class VersionGarbageCollector {
         final Stopwatch active = Stopwatch.createUnstarted();
         final Stopwatch collectDeletedDocs = Stopwatch.createUnstarted();
         final Stopwatch checkDeletedDocs = Stopwatch.createUnstarted();
-        final Stopwatch detailGcDocs = Stopwatch.createUnstarted();
+        final Stopwatch detailedGcDocs = Stopwatch.createUnstarted();
         final Stopwatch deleteDeletedDocs = Stopwatch.createUnstarted();
         final Stopwatch collectAndDeleteSplitDocs = Stopwatch.createUnstarted();
         final Stopwatch deleteSplitDocs = Stopwatch.createUnstarted();
         final Stopwatch sortDocIds = Stopwatch.createUnstarted();
         final Stopwatch updateResurrectedDocuments = Stopwatch.createUnstarted();
         long activeElapsed, collectDeletedDocsElapsed, checkDeletedDocsElapsed, deleteDeletedDocsElapsed, collectAndDeleteSplitDocsElapsed,
-                deleteSplitDocsElapsed, sortDocIdsElapsed, updateResurrectedDocumentsElapsed, detailGcDocsElapsed;
+                deleteSplitDocsElapsed, sortDocIdsElapsed, updateResurrectedDocumentsElapsed, detailedGcDocsElapsed;
 
         @Override
         public String toString() {
@@ -318,6 +324,7 @@ public class VersionGarbageCollector {
 
             return "VersionGCStats{" +
                     "ignoredGCDueToCheckPoint=" + ignoredGCDueToCheckPoint +
+                    ", oldestModifiedGced=" + oldestModifiedGced +
                     ", canceled=" + canceled +
                     ", deletedDocGCCount=" + deletedDocGCCount + " (of which leaf: " + deletedLeafDocGCCount + ")" +
                     ", updateResurrectedGCCount=" + updateResurrectedGCCount +
@@ -331,6 +338,7 @@ public class VersionGarbageCollector {
         void addRun(VersionGCStats run) {
             ++iterationCount;
             this.ignoredGCDueToCheckPoint = run.ignoredGCDueToCheckPoint;
+            this.oldestModifiedGced = run.oldestModifiedGced;
             this.canceled = run.canceled;
             this.success = run.success;
             this.limitExceeded = run.limitExceeded;
@@ -350,7 +358,7 @@ public class VersionGarbageCollector {
                 this.deleteSplitDocsElapsed += run.deleteSplitDocsElapsed;
                 this.sortDocIdsElapsed += run.sortDocIdsElapsed;
                 this.updateResurrectedDocumentsElapsed += run.updateResurrectedDocumentsElapsed;
-                this.detailGcDocsElapsed += run.detailGcDocsElapsed;
+                this.detailedGcDocsElapsed += run.detailedGcDocsElapsed;
             } else {
                 // single run -> read from stop watches
                 this.activeElapsed += run.active.elapsed(MICROSECONDS);
@@ -361,7 +369,7 @@ public class VersionGarbageCollector {
                 this.deleteSplitDocsElapsed += run.deleteSplitDocs.elapsed(MICROSECONDS);
                 this.sortDocIdsElapsed += run.sortDocIds.elapsed(MICROSECONDS);
                 this.updateResurrectedDocumentsElapsed += run.updateResurrectedDocuments.elapsed(MICROSECONDS);
-                this.detailGcDocsElapsed += run.detailGcDocs.elapsed(MICROSECONDS);
+                this.detailedGcDocsElapsed += run.detailedGcDocs.elapsed(MICROSECONDS);
             }
         }
     }
@@ -370,7 +378,7 @@ public class VersionGarbageCollector {
         NONE,
         COLLECTING,
         CHECKING,
-        DETAILGC,
+        DETAILED_GC,
         DELETING,
         SORTING,
         SPLITS_CLEANUP,
@@ -398,7 +406,7 @@ public class VersionGarbageCollector {
             this.watches.put(GCPhase.NONE, Stopwatch.createStarted());
             this.watches.put(GCPhase.COLLECTING, stats.collectDeletedDocs);
             this.watches.put(GCPhase.CHECKING, stats.checkDeletedDocs);
-            this.watches.put(GCPhase.DETAILGC, stats.detailGcDocs);
+            this.watches.put(GCPhase.DETAILED_GC, stats.detailedGcDocs);
             this.watches.put(GCPhase.DELETING, stats.deleteDeletedDocs);
             this.watches.put(GCPhase.SORTING, stats.sortDocIds);
             this.watches.put(GCPhase.SPLITS_CLEANUP, stats.collectAndDeleteSplitDocs);
@@ -525,7 +533,10 @@ public class VersionGarbageCollector {
 
                     collectDeletedDocuments(phases, headRevision, rec);
                     collectSplitDocuments(phases, sweepRevisions, rec);
-                    collectDetailGarbage(phases, headRevision, rec);
+                    if (detailedGCEnabled) {
+                        // run only if enabled
+                        collectDetailedGarbage(phases, headRevision, rec);
+                    }
                 }
             } catch (LimitExceededException ex) {
                 stats.limitExceeded = true;
@@ -555,36 +566,33 @@ public class VersionGarbageCollector {
          * 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 headRevision
+         *
+         * @param phases {@link GCPhases}
+         * @param headRevision the current head revision of
          * @throws IOException
          * @throws LimitExceededException
          */
-        private void collectDetailGarbage(GCPhases phases, RevisionVector headRevision, VersionGCRecommendations rec)
+        private void collectDetailedGarbage(final GCPhases phases, final RevisionVector headRevision, final VersionGCRecommendations rec)
                 throws IOException, LimitExceededException {
-            if (!DETAIL_GC_ENABLED) {
-                // TODO: this toggling should be done nicer asap
-                return;
-            }
             int docsTraversed = 0;
-            DetailGC gc = new DetailGC(headRevision, monitor);
-            try {
-                final long fromModified;
-                final long toModified;
-                if (rec.fullDetailGCTimestamp == -1) {
-                    // then full detail-gc is disabled or over - use regular scope then
-                    fromModified = rec.scope.fromMs;
-                    toModified = rec.scope.toMs;
-                } else {
-                    // then full detail-gc is enabled - use it then
-                    fromModified = rec.fullDetailGCTimestamp; // TODO: once we're passed rec.scope.fromMs we should
-                                                              // disable fullgc
-                    toModified = rec.scope.toMs; // the 'to' here is the max. it will process only eg 1 batch
-                }
-                long oldestGced = fromModified;
-                boolean foundAnything = false;
+            long oldestModifiedGced = rec.scopeFullGC.fromMs;
+            try (DetailedGC gc = new DetailedGC(headRevision, monitor, cancel)) {
+                final long fromModified = rec.scopeFullGC.fromMs;
+                final long toModified = rec.scopeFullGC.toMs;
+//                if (rec.fullDetailGCTimestamp == -1) {
+//                    // then full detail-gc is disabled or over - use regular scope then
+//                    fromModified = rec.scope.fromMs;
+//                    toModified = rec.scope.toMs;
+//                } else {
+//                    // then full detail-gc is enabled - use it then
+//                    fromModified = rec.fullDetailGCTimestamp; // TODO: once we're passed rec.scope.fromMs we should
+//                    // disable fullgc
+//                    toModified = rec.scope.toMs; // the 'to' here is the max. it will process only eg 1 batch
+//                }
+                // TODO : remove me
+                boolean foundAnything = false; // I think this flag is redundant
                 if (phases.start(GCPhase.COLLECTING)) {
-                    Iterable<NodeDocument> itr = versionStore.getModifiedDocs(fromModified, toModified);
+                    Iterable<NodeDocument> itr = versionStore.getModifiedDocs(oldestModifiedGced, toModified, 2000);
                     final Stopwatch timer = Stopwatch.createUnstarted();
                     timer.reset().start();
                     try {
@@ -593,44 +601,54 @@ public class VersionGarbageCollector {
                             if (cancel.get()) {
                                 break;
                             }
-                            foundAnything = true;
-                            if (phases.start(GCPhase.DETAILGC)) {
-                                gc.detailGC(doc, phases);
-                                phases.stop(GCPhase.DETAILGC);
+                            if (phases.start(GCPhase.DETAILED_GC)) {
+                                gc.detailedGC(doc, phases);
+                                phases.stop(GCPhase.DETAILED_GC);
                             }
+
+                            // TODO : remove this code, I don't think its possible to fetch these documents
+                            //  who doesn't have _modified field
                             final Long modified = doc.getModified();
                             if (modified == null) {
                                 monitor.warn("collectDetailGarbage : document has no _modified property : {}",
                                         doc.getId());
-                            } else if (modified < oldestGced) {
+                            } else if (modified < oldestModifiedGced) {
                                 monitor.warn(
                                         "collectDetailGarbage : document has older _modified than query boundary : {} (from: {}, to: {})",
                                         modified, fromModified, toModified);
                             } else {
-                                oldestGced = modified;
+                                oldestModifiedGced = modified;
                             }
+                            foundAnything = true;
                             docsTraversed++;
                             if (docsTraversed % PROGRESS_BATCH_SIZE == 0) {
                                 monitor.info("Iterated through {} documents so far. {} had detail garbage",
                                         docsTraversed, gc.getNumDocuments());
                             }
+                            // this would never hit, since we are only fetching the oldest 2000 element in batches of 1000
+                            // TODO: remove this if above mentioned logic is fine
                             if (rec.maxCollect > 0 && gc.getNumDocuments() > rec.maxCollect) {
                                 // TODO: how would we recover from this?
+                                // If we don't want above solution, then one of the another solution is to use lower time duration
+                                // as done in document deletion process or use lower limit value or
+                                // we should perform all the update ops in 1 go
                                 throw new LimitExceededException();
                             }
+                            oldestModifiedGced = modified == null ? fromModified : modified;
                         }
                     } finally {
                         Utils.closeIfCloseable(itr);
+                        // why do we need to stop this here, we are already stopping the original gc run.
+                        // can this be removed
                         delayOnModifications(timer.stop().elapsed(TimeUnit.MILLISECONDS));
+                        phases.stats.oldestModifiedGced = oldestModifiedGced;
                     }
                     phases.stop(GCPhase.COLLECTING);
-                    if (!cancel.get() && foundAnything) {
-                        // TODO: move to evaluate()
-                        rec.setLongSetting(SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, oldestGced + 1);
-                    }
+//                    if (!cancel.get() && foundAnything) {
+//                        // TODO: move to evaluate()
+//                        rec.setLongSetting(SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, oldestModifiedGced + 1);
+//                    }
                 }
-            } finally {
-                gc.close();
             }
         }
 
@@ -665,8 +683,7 @@ public class VersionGarbageCollector {
                                              VersionGCRecommendations rec)
                 throws IOException, LimitExceededException {
             int docsTraversed = 0;
-            DeletedDocsGC gc = new DeletedDocsGC(headRevision, cancel, options, monitor);
-            try {
+            try (DeletedDocsGC gc = new DeletedDocsGC(headRevision, cancel, options, monitor)) {
                 if (phases.start(GCPhase.COLLECTING)) {
                     Iterable<NodeDocument> itr = versionStore.getPossiblyDeletedDocs(rec.scope.fromMs, rec.scope.toMs);
                     try {
@@ -731,53 +748,69 @@ public class VersionGarbageCollector {
                     gc.updateResurrectedDocuments(phases.stats);
                     phases.stop(GCPhase.UPDATING);
                 }
-            } finally {
-                gc.close();
             }
         }
     }
 
-    private class DetailGC implements Closeable {
+    private static class DetailedGC implements Closeable {
 
         private final RevisionVector headRevision;
         private final GCMonitor monitor;
+        private final AtomicBoolean cancel;
         private int count;
 
-        public DetailGC(@NotNull RevisionVector headRevision, @NotNull GCMonitor monitor) {
-            this.headRevision = checkNotNull(headRevision);
+        public DetailedGC(@NotNull RevisionVector headRevision, @NotNull GCMonitor monitor, @NotNull AtomicBoolean cancel) {
+            this.headRevision = requireNonNull(headRevision);
             this.monitor = monitor;
+            this.cancel = cancel;
         }
 
-        public void detailGC(NodeDocument doc, GCPhases phases) {
-            deleteSample(doc, phases);
-            deleteUnmergedBranchCommitDocument(doc, phases);
-            deleteDeletedProperties(doc, phases);
-            deleteOldRevisions(doc, phases);
+        public void detailedGC(NodeDocument doc, GCPhases phases) {
+//            deleteSample(doc, phases);
+            UpdateOp updateOp = new UpdateOp(requireNonNull(doc.getId()), false);
+            deleteDeletedProperties(doc, phases, updateOp);
+            deleteUnmergedBranchCommitDocument(doc, phases, updateOp);
+            deleteOldRevisions(doc, phases, updateOp);
         }
 
         /** TODO remove, this is just a skeleton sample */
-        private void deleteSample(NodeDocument doc, GCPhases phases) {
-            if (doc.getId().contains("should_delete")) {
-                if (phases.start(GCPhase.DELETING)) {
-                    monitor.info("deleteSample: should do the deletion now, but this is demo only. I'm still learning");
-                    System.out.println("do the actual deletion");
-                    count++;
-                    phases.stop(GCPhase.DELETING);
-                }
-            }
-        }
-
-        private void deleteUnmergedBranchCommitDocument(NodeDocument doc, GCPhases phases) {
+//        private void deleteSample(NodeDocument doc, GCPhases phases) {
+//            if (doc.getId().contains("should_delete")) {
+//                if (phases.start(GCPhase.DELETING)) {
+//                    monitor.info("deleteSample: should do the deletion now, but this is demo only. I'm still learning");
+//                    System.out.println("do the actual deletion");
+//                    count++;
+//                    phases.stop(GCPhase.DELETING);
+//                }
+//            }
+//        }
+
+        private void deleteUnmergedBranchCommitDocument(NodeDocument doc, GCPhases phases, UpdateOp updateOp) {
             // TODO Auto-generated method stub
 
         }
 
-        private void deleteDeletedProperties(NodeDocument doc, GCPhases phases) {
-            // TODO Auto-generated method stub
+        private void deleteDeletedProperties(final NodeDocument doc, final GCPhases phases, final UpdateOp updateOp) {
+
+            // get Map of all properties along with their values
+            final Map<String, SortedMap<Revision, String>> properties = doc.getProperties();
 
+            // find all the properties which can be removed from document
+            // All the properties whose value is null in their respective
+            // latest revision are eligible to be garbage collected.
+            properties.forEach((propName, revisionStringSortedMap) -> {
+                if (revisionStringSortedMap.keySet()
+                        .stream()
+                        .sorted(REVERSE)
+                        .limit(1)
+                        .anyMatch(revision -> revisionStringSortedMap.get(revision) == null)) {
+                    // set this property for removal
+                    updateOp.remove(propName);
+                }
+            });
         }
 
-        private void deleteOldRevisions(NodeDocument doc, GCPhases phases) {
+        private void deleteOldRevisions(NodeDocument doc, GCPhases phases, UpdateOp updateOp) {
             // TODO Auto-generated method stub
 
         }
@@ -813,8 +846,8 @@ public class VersionGarbageCollector {
                              @NotNull AtomicBoolean cancel,
                              @NotNull VersionGCOptions options,
                              @NotNull GCMonitor monitor) {
-            this.headRevision = checkNotNull(headRevision);
-            this.cancel = checkNotNull(cancel);
+            this.headRevision = requireNonNull(headRevision);
+            this.cancel = requireNonNull(cancel);
             this.timer = Stopwatch.createUnstarted();
             this.options = options;
             this.monitor = monitor;
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
index 93f22f5202..e34d8f36b0 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
@@ -22,6 +22,9 @@ package org.apache.jackrabbit.oak.plugins.document.mongo;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.concat;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.transform;
+import static com.mongodb.client.model.Filters.and;
+import static com.mongodb.client.model.Filters.gte;
+import static com.mongodb.client.model.Filters.lt;
 import static java.util.Collections.emptyList;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
 import static org.apache.jackrabbit.oak.plugins.document.Document.ID;
@@ -108,10 +111,10 @@ public class MongoVersionGCSupport extends VersionGCSupport {
     @Override
     public CloseableIterable<NodeDocument> getPossiblyDeletedDocs(final long fromModified, final long toModified) {
         //_deletedOnce == true && _modified >= fromModified && _modified < toModified
-        Bson query = Filters.and(
+        Bson query = and(
                 Filters.eq(DELETED_ONCE, true),
-                Filters.gte(MODIFIED_IN_SECS, getModifiedInSecs(fromModified)),
-                Filters.lt(MODIFIED_IN_SECS, getModifiedInSecs(toModified))
+                gte(MODIFIED_IN_SECS, getModifiedInSecs(fromModified)),
+                lt(MODIFIED_IN_SECS, getModifiedInSecs(toModified))
         );
         FindIterable<BasicDBObject> cursor = getNodeCollection()
                 .find(query).batchSize(batchSize);
@@ -120,6 +123,29 @@ public class MongoVersionGCSupport extends VersionGCSupport {
                 input -> store.convertFromDBObject(NODES, input)));
     }
 
+    /**
+     * Returns documents that have a {@link NodeDocument#MODIFIED_IN_SECS} value
+     * within the given range in sorted order. The two passed modified timestamps
+     * are in milliseconds since the epoch and the implementation will convert them
+     * to seconds at the granularity of the {@link NodeDocument#MODIFIED_IN_SECS}
+     * field and then perform the comparison.
+     *
+     * @param fromModified the lower bound modified timestamp (inclusive)
+     * @param toModified   the upper bound modified timestamp (exclusive)
+     * @return matching documents in sorted order of {@link NodeDocument#MODIFIED_IN_SECS}
+     */
+    @Override
+    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified, final int limit) {
+        // _modified >= fromModified && _modified < toModified
+        final Bson query = and(gte(MODIFIED_IN_SECS, getModifiedInSecs(fromModified)),
+                lt(MODIFIED_IN_SECS, getModifiedInSecs(toModified)));
+        final FindIterable<BasicDBObject> cursor = getNodeCollection()
+                .find(query)
+                .sort(new org.bson.Document(MODIFIED_IN_SECS, 1))
+                .limit(limit);
+        return CloseableIterable.wrap(transform(cursor, input -> store.convertFromDBObject(NODES, input)));
+    }
+
     @Override
     public long getDeletedOnceCount() {
         Bson query = Filters.eq(DELETED_ONCE, Boolean.TRUE);
@@ -207,9 +233,9 @@ public class MongoVersionGCSupport extends VersionGCSupport {
         }
         // OAK-8351: this (last) query only contains SD_TYPE and SD_MAX_REV_TIME_IN_SECS
         // so mongodb should really use that _sdType_1__sdMaxRevTime_1 index
-        result.add(Filters.and(
+        result.add(and(
                 Filters.or(orClauses),
-                Filters.lt(SD_MAX_REV_TIME_IN_SECS, getModifiedInSecs(oldestRevTimeStamp))
+                lt(SD_MAX_REV_TIME_IN_SECS, getModifiedInSecs(oldestRevTimeStamp))
                 ));
 
         return result;
@@ -240,16 +266,16 @@ public class MongoVersionGCSupport extends VersionGCSupport {
             Bson idPathClause = Filters.or(
                     Filters.regex(ID, Pattern.compile(".*" + idSuffix)),
                     // previous documents with long paths do not have a '-' in the id
-                    Filters.and(
+                    and(
                             Filters.regex(ID, Pattern.compile("[^-]*")),
                             Filters.regex(PATH, Pattern.compile(".*" + idSuffix))
                     )
             );
 
             long minMaxRevTimeInSecs = Math.min(maxRevTimeInSecs, getModifiedInSecs(r.getTimestamp()));
-            result.add(Filters.and(
+            result.add(and(
                     Filters.eq(SD_TYPE, DEFAULT_NO_BRANCH.typeCode()),
-                    Filters.lt(SD_MAX_REV_TIME_IN_SECS, minMaxRevTimeInSecs),
+                    lt(SD_MAX_REV_TIME_IN_SECS, minMaxRevTimeInSecs),
                     idPathClause
                     ));
         }
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
index 793e3a9433..c9428429bc 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
@@ -894,6 +894,17 @@ public class Utils {
         return builder.isThrottlingEnabled() || (docStoreThrottlingFeature != null && docStoreThrottlingFeature.isEnabled());
     }
 
+    /**
+     * Check whether detailed GC is enabled or not for document store.
+     *
+     * @param builder instance for DocumentNodeStoreBuilder
+     * @return true if detailed GC is enabled else false
+     */
+    public static boolean isDetailedGCEnabled(final DocumentNodeStoreBuilder<?> builder) {
+        final Feature docStoreDetailedGCFeature = builder.getDocStoreDetailedGCFeature();
+        return builder.isDetailedGCEnabled() || (docStoreDetailedGCFeature != null && docStoreDetailedGCFeature.isEnabled());
+    }
+
     /**
      * Returns true if all the revisions in the {@code a} greater or equals
      * to their counterparts in {@code b}. If {@code b} contains revisions
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java
index c46b6f699e..88d9d27553 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java
@@ -33,6 +33,7 @@ import org.osgi.framework.BundleContext;
 import org.osgi.service.cm.ConfigurationAdmin;
 import org.osgi.service.component.ComponentContext;
 
+import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.DEFAULT_DETAILED_GC_ENABLED;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.DEFAULT_THROTTLING_ENABLED;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCQueryTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCQueryTest.java
index 73eda05759..27333b57cf 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCQueryTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCQueryTest.java
@@ -107,7 +107,7 @@ public class VersionGCQueryTest {
         clock.waitUntil(clock.getTime() + TimeUnit.HOURS.toMillis(1));
 
         VersionGarbageCollector gc = new VersionGarbageCollector(
-                ns, new VersionGCSupport(store));
+                ns, new VersionGCSupport(store), false);
         prevDocIds.clear();
         VersionGCStats stats = gc.gc(30, TimeUnit.MINUTES);
         assertEquals(11, stats.deletedDocGCCount);
@@ -140,7 +140,7 @@ public class VersionGCQueryTest {
         clock.waitUntil(clock.getTime() + TimeUnit.HOURS.toMillis(1));
 
         VersionGarbageCollector gc = new VersionGarbageCollector(
-                ns, new VersionGCSupport(store));
+                ns, new VersionGCSupport(store), false);
         prevDocIds.clear();
         VersionGCStats stats = gc.gc(30, TimeUnit.MINUTES);
         assertEquals(1, stats.deletedDocGCCount);
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
index 445e7c4275..48f3f362ce 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
@@ -44,6 +44,7 @@ import org.jetbrains.annotations.NotNull;
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 
@@ -324,7 +325,7 @@ public class VersionGCTest {
                 deletedOnceCountCalls.incrementAndGet();
                 return Iterables.size(Utils.getSelectedDocuments(store, NodeDocument.DELETED_ONCE, 1));
             }
-        });
+        }, false);
 
         // run first RGC
         gc.gc(1, TimeUnit.HOURS);
@@ -342,21 +343,25 @@ public class VersionGCTest {
 
     // OAK-10199
     @Test
+    @Ignore
     public void testDetailGcDocumentRead_disabled() throws Exception {
         DetailGCHelper.disableDetailGC(ns);
         VersionGCStats stats = gc.gc(30, TimeUnit.MINUTES);
         assertNotNull(stats);
-        assertEquals(0, stats.detailGcDocsElapsed);
+        assertEquals(0, stats.detailedGcDocsElapsed);
     }
 
     @Test
+    @Ignore
     public void testDetailGcDocumentRead_enabled() throws Exception {
         DetailGCHelper.enableDetailGC(ns);
         VersionGCStats stats = gc.gc(30, TimeUnit.MINUTES);
         assertNotNull(stats);
-        assertNotEquals(0, stats.detailGcDocsElapsed);
+        assertNotEquals(0, stats.detailedGcDocsElapsed);
     }
 
+    // OAK-10199
+
     private Future<VersionGCStats> gc() {
         // run gc in a separate thread
         return execService.submit(new Callable<VersionGCStats>() {
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
index e58741733a..d33cc8c7de 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
@@ -497,7 +497,7 @@ public class VersionGarbageCollectorIT {
                         });
             }
         };
-        final VersionGarbageCollector gc = new VersionGarbageCollector(store, gcSupport);
+        final VersionGarbageCollector gc = new VersionGarbageCollector(store, gcSupport, false);
         // start GC -> will try to remove /foo and /bar
         Future<VersionGCStats> f = execService.submit(new Callable<VersionGCStats>() {
             @Override
@@ -658,7 +658,7 @@ public class VersionGarbageCollectorIT {
                 return super.getPossiblyDeletedDocs(fromModified, toModified);
             }
         };
-        gcRef.set(new VersionGarbageCollector(store, gcSupport));
+        gcRef.set(new VersionGarbageCollector(store, gcSupport, false));
         VersionGCStats stats = gcRef.get().gc(30, TimeUnit.MINUTES);
         assertTrue(stats.canceled);
         assertEquals(0, stats.deletedDocGCCount);
@@ -710,7 +710,7 @@ public class VersionGarbageCollectorIT {
                 return super.getPossiblyDeletedDocs(prevLastModifiedTime, lastModifiedTime).iterator();
             }
         };
-        gcRef.set(new VersionGarbageCollector(store, gcSupport));
+        gcRef.set(new VersionGarbageCollector(store, gcSupport, false));
         VersionGCStats stats = gcRef.get().gc(30, TimeUnit.MINUTES);
         assertTrue(stats.canceled);
         assertEquals(0, stats.deletedDocGCCount);
@@ -739,7 +739,7 @@ public class VersionGarbageCollectorIT {
                         });
             }
         };
-        final VersionGarbageCollector gc = new VersionGarbageCollector(store, nonReportingGcSupport);
+        final VersionGarbageCollector gc = new VersionGarbageCollector(store, nonReportingGcSupport, false);
         final long maxAgeHours = 1;
         final long clockDelta = HOURS.toMillis(maxAgeHours) + MINUTES.toMillis(5);
 
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentNodeStoreBuilderTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentNodeStoreBuilderTest.java
index a78aef904e..a08abe05d3 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentNodeStoreBuilderTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentNodeStoreBuilderTest.java
@@ -48,6 +48,18 @@ public class MongoDocumentNodeStoreBuilderTest {
         assertNull(builder.getDocStoreThrottlingFeature());
     }
 
+    @Test
+    public void detailedGCDisabled() {
+        MongoDocumentNodeStoreBuilder builder = new MongoDocumentNodeStoreBuilder();
+        assertFalse(builder.isDetailedGCEnabled());
+    }
+
+    @Test
+    public void detailedGCFeatureToggleDisabled() {
+        MongoDocumentNodeStoreBuilder builder = new MongoDocumentNodeStoreBuilder();
+        assertNull(builder.getDocStoreDetailedGCFeature());
+    }
+
     @Test
     public void collectionCompressionDisabled() {
         MongoDocumentNodeStoreBuilder builder = new MongoDocumentNodeStoreBuilder();
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
index 2b01f90b34..6041a41724 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
@@ -50,13 +50,13 @@ import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.toggle.Feature;
 import org.apache.jackrabbit.oak.stats.Clock;
-import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.mockito.Mockito;
 import org.slf4j.event.Level;
 
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreBuilder.newDocumentNodeStoreBuilder;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.isDetailedGCEnabled;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.isThrottlingEnabled;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
@@ -184,6 +184,45 @@ public class UtilsTest {
         assertTrue("Throttling is enabled via Feature Toggle", throttlingEnabled);
     }
 
+    @Test
+    public void detailedGCEnabledDefaultValue() {
+        boolean detailedGCEnabled = isDetailedGCEnabled(newDocumentNodeStoreBuilder());
+        assertFalse("Detailed GC is disabled by default", detailedGCEnabled);
+    }
+
+    @Test
+    public void detailedGCExplicitlyDisabled() {
+        DocumentNodeStoreBuilder<?> builder = newDocumentNodeStoreBuilder();
+        builder.setDetailedGCEnabled(false);
+        Feature docStoreDetailedGCFeature = mock(Feature.class);
+        when(docStoreDetailedGCFeature.isEnabled()).thenReturn(false);
+        builder.setDocStoreDetailedGCFeature(docStoreDetailedGCFeature);
+        boolean detailedGCEnabled = isDetailedGCEnabled(builder);
+        assertFalse("Detailed GC is disabled explicitly", detailedGCEnabled);
+    }
+
+    @Test
+    public void detailedGCEnabledViaConfiguration() {
+        DocumentNodeStoreBuilder<?> builder = newDocumentNodeStoreBuilder();
+        builder.setDetailedGCEnabled(true);
+        Feature docStoreDetailedGCFeature = mock(Feature.class);
+        when(docStoreDetailedGCFeature.isEnabled()).thenReturn(false);
+        builder.setDocStoreDetailedGCFeature(docStoreDetailedGCFeature);
+        boolean detailedGCEnabled = isDetailedGCEnabled(builder);
+        assertTrue("Detailed GC is enabled via configuration", detailedGCEnabled);
+    }
+
+    @Test
+    public void detailedGCEnabledViaFeatureToggle() {
+        DocumentNodeStoreBuilder<?> builder = newDocumentNodeStoreBuilder();
+        builder.setDetailedGCEnabled(false);
+        Feature docStoreDetailedGCFeature = mock(Feature.class);
+        when(docStoreDetailedGCFeature.isEnabled()).thenReturn(true);
+        builder.setDocStoreDetailedGCFeature(docStoreDetailedGCFeature);
+        boolean detailedGCEnabled = isDetailedGCEnabled(builder);
+        assertTrue("Detailed GC is enabled via Feature Toggle", detailedGCEnabled);
+    }
+
     @Test
     public void getDepthFromId() throws Exception{
         assertEquals(1, Utils.getDepthFromId("1:/x"));


[jackrabbit-oak] 01/08: OAK-10199 : initial sketch of detail gc skeleton

Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

daim pushed a commit to branch OAK-10199
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit 6e25ec594d35de314fc9a8f41dfd53c0800e095a
Author: Stefan Egli <st...@apache.org>
AuthorDate: Thu Apr 20 18:11:08 2023 +0200

    OAK-10199 : initial sketch of detail gc skeleton
---
 .../plugins/document/VersionGCRecommendations.java |   6 +-
 .../oak/plugins/document/VersionGCSupport.java     |  25 +++
 .../plugins/document/VersionGarbageCollector.java  | 183 ++++++++++++++++++++-
 .../oak/plugins/document/DetailGCHelper.java       |  42 +++++
 .../oak/plugins/document/VersionGCTest.java        |  18 ++
 5 files changed, 272 insertions(+), 2 deletions(-)

diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
index 363c65789b..ac47cc69d8 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
@@ -46,6 +46,7 @@ public class VersionGCRecommendations {
     final long maxCollect;
     final long deleteCandidateCount;
     final long lastOldestTimestamp;
+    final long fullDetailGCTimestamp;
     final long originalCollectLimit;
 
     private final long precisionMs;
@@ -101,6 +102,8 @@ public class VersionGCRecommendations {
         TimeInterval scope = new TimeInterval(oldestPossible, Long.MAX_VALUE);
         scope = scope.notLaterThan(keep.fromMs);
 
+        fullDetailGCTimestamp = settings.get(VersionGarbageCollector.SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP);
+
         suggestedIntervalMs = settings.get(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP);
         if (suggestedIntervalMs > 0) {
             suggestedIntervalMs = Math.max(suggestedIntervalMs, options.precisionMs);
@@ -217,6 +220,7 @@ public class VersionGCRecommendations {
         // default values
         settings.put(VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, 0L);
         settings.put(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, 0L);
+        settings.put(VersionGarbageCollector.SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, -1L);
         if (versionGCDoc != null) {
             for (String k : versionGCDoc.keySet()) {
                 Object value = versionGCDoc.get(k);
@@ -228,7 +232,7 @@ public class VersionGCRecommendations {
         return settings;
     }
 
-    private void setLongSetting(String propName, long val) {
+    void setLongSetting(String propName, long val) {
         UpdateOp updateOp = new UpdateOp(VersionGarbageCollector.SETTINGS_COLLECTION_ID, true);
         updateOp.set(propName, val);
         vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java
index 13171b7fd5..0e5c26c83d 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java
@@ -83,6 +83,31 @@ public class VersionGCSupport {
         });
     }
 
+    /**
+     * TODO: document me!
+     */
+    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified) {
+        return filter(getSelectedDocuments(store, NodeDocument.MODIFIED_IN_SECS, fromModified), new Predicate<NodeDocument>() {
+            @Override
+            public boolean apply(NodeDocument input) {
+                return modifiedGreaterThanEquals(input, fromModified)
+                        && modifiedLessThan(input, toModified);
+            }
+
+            private boolean modifiedGreaterThanEquals(NodeDocument doc,
+                                                      long time) {
+                Long modified = doc.getModified();
+                return modified != null && modified.compareTo(getModifiedInSecs(time)) >= 0;
+            }
+
+            private boolean modifiedLessThan(NodeDocument doc,
+                                             long time) {
+                Long modified = doc.getModified();
+                return modified != null && modified.compareTo(getModifiedInSecs(time)) < 0;
+            }
+        });
+    }
+
     /**
      * Returns the underlying document store.
      *
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
index ed0b333d44..e7442a7d15 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
@@ -70,6 +70,9 @@ import static org.slf4j.helpers.MessageFormatter.arrayFormat;
 
 public class VersionGarbageCollector {
 
+    /** TODO temporary global flag to enable 'detail gc' during prototyping. Should eventually become eg a system property */
+    public static boolean DETAIL_GC_ENABLED = false;
+
     //Kept less than MongoDocumentStore.IN_CLAUSE_BATCH_SIZE to avoid re-partitioning
     private static final int DELETE_BATCH_SIZE = 450;
     private static final int UPDATE_BATCH_SIZE = 450;
@@ -99,6 +102,17 @@ public class VersionGarbageCollector {
      */
     static final String SETTINGS_COLLECTION_REC_INTERVAL_PROP = "recommendedIntervalMs";
 
+    /**
+     * Property name to timestamp when last full-detail-GC run happened, or -1 if not applicable/in-use.
+     * <p>
+     * <ul>
+     * <li>-1 : full repo scan is disabled</li>
+     * <li>0 : full repo scan is enabled and bound to start from zero == oldest _modified </li>
+     * <li>gt 0 : full repo scan is enabled, was already done up until this value</li>
+     * </ul>
+     */
+    static final String SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP = "fullDetailGCTimeStamp";
+
     private final DocumentNodeStore nodeStore;
     private final DocumentStore ds;
     private final VersionGCSupport versionStore;
@@ -260,13 +274,14 @@ public class VersionGarbageCollector {
         final Stopwatch active = Stopwatch.createUnstarted();
         final Stopwatch collectDeletedDocs = Stopwatch.createUnstarted();
         final Stopwatch checkDeletedDocs = Stopwatch.createUnstarted();
+        final Stopwatch detailGcDocs = Stopwatch.createUnstarted();
         final Stopwatch deleteDeletedDocs = Stopwatch.createUnstarted();
         final Stopwatch collectAndDeleteSplitDocs = Stopwatch.createUnstarted();
         final Stopwatch deleteSplitDocs = Stopwatch.createUnstarted();
         final Stopwatch sortDocIds = Stopwatch.createUnstarted();
         final Stopwatch updateResurrectedDocuments = Stopwatch.createUnstarted();
         long activeElapsed, collectDeletedDocsElapsed, checkDeletedDocsElapsed, deleteDeletedDocsElapsed, collectAndDeleteSplitDocsElapsed,
-                deleteSplitDocsElapsed, sortDocIdsElapsed, updateResurrectedDocumentsElapsed;
+                deleteSplitDocsElapsed, sortDocIdsElapsed, updateResurrectedDocumentsElapsed, detailGcDocsElapsed;
 
         @Override
         public String toString() {
@@ -335,6 +350,7 @@ public class VersionGarbageCollector {
                 this.deleteSplitDocsElapsed += run.deleteSplitDocsElapsed;
                 this.sortDocIdsElapsed += run.sortDocIdsElapsed;
                 this.updateResurrectedDocumentsElapsed += run.updateResurrectedDocumentsElapsed;
+                this.detailGcDocsElapsed += run.detailGcDocsElapsed;
             } else {
                 // single run -> read from stop watches
                 this.activeElapsed += run.active.elapsed(MICROSECONDS);
@@ -345,6 +361,7 @@ public class VersionGarbageCollector {
                 this.deleteSplitDocsElapsed += run.deleteSplitDocs.elapsed(MICROSECONDS);
                 this.sortDocIdsElapsed += run.sortDocIds.elapsed(MICROSECONDS);
                 this.updateResurrectedDocumentsElapsed += run.updateResurrectedDocuments.elapsed(MICROSECONDS);
+                this.detailGcDocsElapsed += run.detailGcDocs.elapsed(MICROSECONDS);
             }
         }
     }
@@ -353,6 +370,7 @@ public class VersionGarbageCollector {
         NONE,
         COLLECTING,
         CHECKING,
+        DETAILGC,
         DELETING,
         SORTING,
         SPLITS_CLEANUP,
@@ -380,6 +398,7 @@ public class VersionGarbageCollector {
             this.watches.put(GCPhase.NONE, Stopwatch.createStarted());
             this.watches.put(GCPhase.COLLECTING, stats.collectDeletedDocs);
             this.watches.put(GCPhase.CHECKING, stats.checkDeletedDocs);
+            this.watches.put(GCPhase.DETAILGC, stats.detailGcDocs);
             this.watches.put(GCPhase.DELETING, stats.deleteDeletedDocs);
             this.watches.put(GCPhase.SORTING, stats.sortDocIds);
             this.watches.put(GCPhase.SPLITS_CLEANUP, stats.collectAndDeleteSplitDocs);
@@ -506,6 +525,7 @@ public class VersionGarbageCollector {
 
                     collectDeletedDocuments(phases, headRevision, rec);
                     collectSplitDocuments(phases, sweepRevisions, rec);
+                    collectDetailGarbage(phases, headRevision, rec);
                 }
             } catch (LimitExceededException ex) {
                 stats.limitExceeded = true;
@@ -521,6 +541,112 @@ public class VersionGarbageCollector {
             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 headRevision
+         * @throws IOException
+         * @throws LimitExceededException
+         */
+        private void collectDetailGarbage(GCPhases phases, RevisionVector headRevision, VersionGCRecommendations rec)
+                throws IOException, LimitExceededException {
+            if (!DETAIL_GC_ENABLED) {
+                // TODO: this toggling should be done nicer asap
+                return;
+            }
+            int docsTraversed = 0;
+            DetailGC gc = new DetailGC(headRevision, monitor);
+            try {
+                final long fromModified;
+                final long toModified;
+                if (rec.fullDetailGCTimestamp == -1) {
+                    // then full detail-gc is disabled or over - use regular scope then
+                    fromModified = rec.scope.fromMs;
+                    toModified = rec.scope.toMs;
+                } else {
+                    // then full detail-gc is enabled - use it then
+                    fromModified = rec.fullDetailGCTimestamp; // TODO: once we're passed rec.scope.fromMs we should
+                                                              // disable fullgc
+                    toModified = rec.scope.toMs; // the 'to' here is the max. it will process only eg 1 batch
+                }
+                long oldestGced = fromModified;
+                boolean foundAnything = false;
+                if (phases.start(GCPhase.COLLECTING)) {
+                    Iterable<NodeDocument> itr = versionStore.getModifiedDocs(fromModified, toModified);
+                    final Stopwatch timer = Stopwatch.createUnstarted();
+                    timer.reset().start();
+                    try {
+                        for (NodeDocument doc : itr) {
+                            // continue with GC?
+                            if (cancel.get()) {
+                                break;
+                            }
+                            foundAnything = true;
+                            if (phases.start(GCPhase.DETAILGC)) {
+                                gc.detailGC(doc, phases);
+                                phases.stop(GCPhase.DETAILGC);
+                            }
+                            final Long modified = doc.getModified();
+                            if (modified == null) {
+                                monitor.warn("collectDetailGarbage : document has no _modified property : {}",
+                                        doc.getId());
+                            } else if (modified < oldestGced) {
+                                monitor.warn(
+                                        "collectDetailGarbage : document has older _modified than query boundary : {} (from: {}, to: {})",
+                                        modified, fromModified, toModified);
+                            } else {
+                                oldestGced = modified;
+                            }
+                            docsTraversed++;
+                            if (docsTraversed % PROGRESS_BATCH_SIZE == 0) {
+                                monitor.info("Iterated through {} documents so far. {} had detail garbage",
+                                        docsTraversed, gc.getNumDocuments());
+                            }
+                            if (rec.maxCollect > 0 && gc.getNumDocuments() > rec.maxCollect) {
+                                // TODO: how would we recover from this?
+                                throw new LimitExceededException();
+                            }
+                        }
+                    } finally {
+                        Utils.closeIfCloseable(itr);
+                        delayOnModifications(timer.stop().elapsed(TimeUnit.MILLISECONDS));
+                    }
+                    phases.stop(GCPhase.COLLECTING);
+                    if (!cancel.get() && foundAnything) {
+                        // TODO: move to evaluate()
+                        rec.setLongSetting(SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, oldestGced + 1);
+                    }
+                }
+            } finally {
+                gc.close();
+            }
+        }
+
+        private void delayOnModifications(long durationMs) {
+            long delayMs = Math.round(durationMs * options.delayFactor);
+            if (!cancel.get() && delayMs > 0) {
+                try {
+                    Clock clock = nodeStore.getClock();
+                    clock.waitUntil(clock.getTime() + delayMs);
+                }
+                catch (InterruptedException ex) {
+                    /* ignore */
+                }
+            }
+        }
+
         private void collectSplitDocuments(GCPhases phases,
                                            RevisionVector sweepRevisions,
                                            VersionGCRecommendations rec) {
@@ -611,6 +737,61 @@ public class VersionGarbageCollector {
         }
     }
 
+    private class DetailGC implements Closeable {
+
+        private final RevisionVector headRevision;
+        private final GCMonitor monitor;
+        private int count;
+
+        public DetailGC(@NotNull RevisionVector headRevision, @NotNull GCMonitor monitor) {
+            this.headRevision = checkNotNull(headRevision);
+            this.monitor = monitor;
+        }
+
+        public void detailGC(NodeDocument doc, GCPhases phases) {
+            deleteSample(doc, phases);
+            deleteUnmergedBranchCommitDocument(doc, phases);
+            deleteDeletedProperties(doc, phases);
+            deleteOldRevisions(doc, phases);
+        }
+
+        /** TODO remove, this is just a skeleton sample */
+        private void deleteSample(NodeDocument doc, GCPhases phases) {
+            if (doc.getId().contains("should_delete")) {
+                if (phases.start(GCPhase.DELETING)) {
+                    monitor.info("deleteSample: should do the deletion now, but this is demo only. I'm still learning");
+                    System.out.println("do the actual deletion");
+                    count++;
+                    phases.stop(GCPhase.DELETING);
+                }
+            }
+        }
+
+        private void deleteUnmergedBranchCommitDocument(NodeDocument doc, GCPhases phases) {
+            // TODO Auto-generated method stub
+
+        }
+
+        private void deleteDeletedProperties(NodeDocument doc, GCPhases phases) {
+            // TODO Auto-generated method stub
+
+        }
+
+        private void deleteOldRevisions(NodeDocument doc, GCPhases phases) {
+            // TODO Auto-generated method stub
+
+        }
+
+        long getNumDocuments() {
+            return count;
+        }
+
+        @Override
+        public void close() throws IOException {
+
+        }
+    }
+
     /**
      * A helper class to remove document for deleted nodes.
      */
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DetailGCHelper.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DetailGCHelper.java
new file mode 100644
index 0000000000..8a585c7dc0
--- /dev/null
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DetailGCHelper.java
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document;
+
+public class DetailGCHelper {
+
+    public static void setLongSetting(String propName, long val, DocumentNodeStore ns) {
+        UpdateOp updateOp = new UpdateOp(VersionGarbageCollector.SETTINGS_COLLECTION_ID, true);
+        updateOp.set(propName, val);
+        ns.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
+    }
+
+    public static void enableDetailGC(DocumentNodeStore ns) {
+        VersionGarbageCollector.DETAIL_GC_ENABLED = true;
+        if (ns != null) {
+            setLongSetting(VersionGarbageCollector.SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, 0, ns);
+        }
+    }
+
+    public static void disableDetailGC(DocumentNodeStore ns) {
+        VersionGarbageCollector.DETAIL_GC_ENABLED = false;
+        if (ns != null) {
+            setLongSetting(VersionGarbageCollector.SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, -1, ns);
+        }
+    }
+}
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
index a3e7a5e1e3..1bd81ce89c 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
@@ -51,6 +51,7 @@ import static java.util.concurrent.TimeUnit.HOURS;
 import static java.util.concurrent.TimeUnit.MINUTES;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -338,6 +339,23 @@ public class VersionGCTest {
         }
     }
 
+    // OAK-10199
+    @Test
+    public void testDetailGcDocumentRead_disabled() throws Exception {
+        DetailGCHelper.disableDetailGC(ns);
+        VersionGCStats stats = gc.gc(30, TimeUnit.MINUTES);
+        assertNotNull(stats);
+        assertEquals(0, stats.detailGcDocsElapsed);
+    }
+
+    @Test
+    public void testDetailGcDocumentRead_enabled() throws Exception {
+        DetailGCHelper.enableDetailGC(ns);
+        VersionGCStats stats = gc.gc(30, TimeUnit.MINUTES);
+        assertNotNull(stats);
+        assertNotEquals(0, stats.detailGcDocsElapsed);
+    }
+
     private Future<VersionGCStats> gc() {
         // run gc in a separate thread
         return execService.submit(new Callable<VersionGCStats>() {


[jackrabbit-oak] 05/08: OAK-10199 : used bulk findAndModify api to perform garbage cleanup

Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

daim pushed a commit to branch OAK-10199
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit 2431421d11b441da22460aee946041fcf9c80ada
Author: Rishabh Kumar <di...@adobe.com>
AuthorDate: Tue May 30 19:38:02 2023 +0530

    OAK-10199 : used bulk findAndModify api to perform garbage cleanup
---
 .../oak/plugins/document/Configuration.java         |  8 ++++++++
 .../plugins/document/DocumentNodeStoreBuilder.java  | 21 +++++++++++++++++++++
 .../plugins/document/DocumentNodeStoreService.java  |  2 ++
 .../plugins/document/VersionGarbageCollector.java   |  5 +----
 .../DocumentNodeStoreServiceConfigurationTest.java  |  9 +++++++++
 5 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java
index ae7aa143d2..52bf2ecc30 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java
@@ -290,4 +290,12 @@ import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreServic
                     "a change that is not yet visible. Default: " + DEFAULT_SUSPEND_TIMEOUT +
                     " (milliseconds).")
     long suspendTimeoutMillis() default DEFAULT_SUSPEND_TIMEOUT;
+
+    @AttributeDefinition(
+            name = "Document Node Store Detailed GC",
+            description = "Boolean value indicating whether Detailed GC should be enabled for " +
+                    "document node store or not. The Default value is " + DEFAULT_DETAILED_GC_ENABLED +
+                    ". Note that this value can be overridden via framework " +
+                    "property 'oak.documentstore.detailedGCEnabled'")
+    boolean detailedGCEnabled() default DEFAULT_DETAILED_GC_ENABLED;
 }
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
index aa3ab1ea81..d894aa27e4 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
@@ -164,6 +164,7 @@ public class DocumentNodeStoreBuilder<T extends DocumentNodeStoreBuilder<T>> {
     private Predicate<Path> nodeCachePredicate = Predicates.alwaysTrue();
     private boolean clusterInvisible;
     private boolean throttlingEnabled;
+    private boolean detailedGCEnabled;
     private long suspendTimeoutMillis = DEFAULT_SUSPEND_TIMEOUT;
 
     /**
@@ -287,6 +288,16 @@ public class DocumentNodeStoreBuilder<T extends DocumentNodeStoreBuilder<T>> {
         return this.throttlingEnabled;
     }
 
+    public T setDetailedGCEnabled(boolean b) {
+        this.detailedGCEnabled = b;
+        return thisBuilder();
+    }
+
+    public boolean isDetailedGCEnabled() {
+        return this.detailedGCEnabled;
+    }
+
+
     public T setReadOnlyMode() {
         this.isReadOnlyMode = true;
         return thisBuilder();
@@ -316,6 +327,16 @@ public class DocumentNodeStoreBuilder<T extends DocumentNodeStoreBuilder<T>> {
         return docStoreThrottlingFeature;
     }
 
+    public T setDocStoreDetailedGCFeature(@Nullable Feature docStoreDetailedGC) {
+        this.docStoreDetailedGCFeature = docStoreDetailedGC;
+        return thisBuilder();
+    }
+
+    @Nullable
+    public Feature getDocStoreDetailedGCFeature() {
+        return docStoreDetailedGCFeature;
+    }
+
     public T setLeaseFailureHandler(LeaseFailureHandler leaseFailureHandler) {
         this.leaseFailureHandler = leaseFailureHandler;
         return thisBuilder();
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
index ad01cda3ca..16229db0e1 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
@@ -473,7 +473,9 @@ public class DocumentNodeStoreService {
                 setLeaseCheckMode(ClusterNodeInfo.DEFAULT_LEASE_CHECK_DISABLED ? LeaseCheckMode.DISABLED : LeaseCheckMode.valueOf(config.leaseCheckMode())).
                 setPrefetchFeature(prefetchFeature).
                 setDocStoreThrottlingFeature(docStoreThrottlingFeature).
+                setDocStoreDetailedGCFeature(docStoreDetailedGCFeature).
                 setThrottlingEnabled(config.throttlingEnabled()).
+                setDetailedGCEnabled(config.detailedGCEnabled()).
                 setSuspendTimeoutMillis(config.suspendTimeoutMillis()).
                 setLeaseFailureHandler(new LeaseFailureHandler() {
 
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
index 27ee36204a..3f9cb23f9a 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
@@ -635,8 +635,6 @@ public class VersionGarbageCollector {
                                     phases.stop(GCPhase.COLLECTING);
                                 }
 
-                                // TODO : remove this code, I don't think its possible to fetch these documents
-                                //  who doesn't have _modified field
                                 final Long modified = doc.getModified();
                                 if (modified == null) {
                                     monitor.warn("collectDetailGarbage : document has no _modified property : {}",
@@ -874,8 +872,7 @@ public class VersionGarbageCollector {
 
             timer.reset().start();
             try {
-                // TODO create an api to bulk update findAndUpdate Ops
-                updatedDocs = (int) updateOpList.stream().map(op -> ds.findAndUpdate(NODES, op)).filter(Objects::nonNull).count();
+                updatedDocs = (int) ds.findAndUpdate(NODES, updateOpList).stream().filter(Objects::nonNull).count();
                 stats.updatedDetailedGCDocsCount += updatedDocs;
                 log.info("Updated [{}] documents", updatedDocs);
                 // now reset delete metadata
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java
index 88d9d27553..b3d5d69fb3 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java
@@ -85,6 +85,7 @@ public class DocumentNodeStoreServiceConfigurationTest {
         assertEquals(Arrays.asList("/"), Arrays.asList(config.persistentCacheIncludes()));
         assertEquals("STRICT", config.leaseCheckMode());
         assertEquals(DEFAULT_THROTTLING_ENABLED, config.throttlingEnabled());
+        assertEquals(DEFAULT_DETAILED_GC_ENABLED, config.detailedGCEnabled());
         assertEquals(CommitQueue.DEFAULT_SUSPEND_TIMEOUT, config.suspendTimeoutMillis());
     }
 
@@ -104,6 +105,14 @@ public class DocumentNodeStoreServiceConfigurationTest {
         assertEquals(throttleDocStore, config.throttlingEnabled());
     }
 
+    @Test
+    public void detailedGCEnabled() throws Exception {
+        boolean detailedGCDocStore = true;
+        addConfigurationEntry(preset, "detailedGCEnabled", detailedGCDocStore);
+        Configuration config = createConfiguration();
+        assertEquals(detailedGCDocStore, config.detailedGCEnabled());
+    }
+
     @Test
     public void presetSocketKeepAlive() throws Exception {
         boolean keepAlive = !DocumentNodeStoreService.DEFAULT_SO_KEEP_ALIVE;


[jackrabbit-oak] 07/08: OAK-10199 : fixed code smells as suggested by Sonar

Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

daim pushed a commit to branch OAK-10199
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit 2a330be5e4b2b46cfe94479badbcbc21f948b172
Author: Rishabh Kumar <di...@adobe.com>
AuthorDate: Wed May 31 14:33:10 2023 +0530

    OAK-10199 : fixed code smells as suggested by Sonar
---
 .../oak/plugins/document/VersionGCRecommendations.java    | 13 +++++++------
 .../oak/plugins/document/VersionGarbageCollector.java     | 15 +++++++--------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
index f04b56fc52..4584d925c0 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
@@ -22,7 +22,6 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
-import com.google.common.collect.ImmutableMap;
 import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats;
 import org.apache.jackrabbit.oak.plugins.document.util.TimeInterval;
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
@@ -31,6 +30,8 @@ import org.apache.jackrabbit.oak.stats.Clock;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static java.lang.Long.MAX_VALUE;
+import static java.util.Map.of;
 import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP;
 import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP;
 
@@ -120,8 +121,8 @@ public class VersionGCRecommendations {
             oldestPossibleFullGC = detailedGCTimestamp - 1;
         }
 
-        TimeInterval scopeFullGC = new TimeInterval(oldestPossibleFullGC, Long.MAX_VALUE);
-        scopeFullGC = scopeFullGC.notLaterThan(keep.fromMs);
+        TimeInterval fullGCTimeInternal = new TimeInterval(oldestPossibleFullGC, MAX_VALUE);
+        fullGCTimeInternal = fullGCTimeInternal.notLaterThan(keep.fromMs);
 
         suggestedIntervalMs = settings.get(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP);
         if (suggestedIntervalMs > 0) {
@@ -181,7 +182,7 @@ public class VersionGCRecommendations {
         this.precisionMs = options.precisionMs;
         this.ignoreDueToCheckPoint = ignoreDueToCheckPoint;
         this.scope = scope;
-        this.scopeFullGC = scopeFullGC;
+        this.scopeFullGC = fullGCTimeInternal;
         this.scopeIsComplete = scope.toMs >= keep.fromMs;
         this.maxCollect = collectLimit;
         this.suggestedIntervalMs = suggestedIntervalMs;
@@ -205,7 +206,7 @@ public class VersionGCRecommendations {
             stats.needRepeat = true;
         } else if (!stats.canceled && !stats.ignoredGCDueToCheckPoint) {
             // success, we would not expect to encounter revisions older than this in the future
-            setLongSetting(ImmutableMap.of(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs,
+            setLongSetting(of(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs,
                     SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP, stats.oldestModifiedGced));
 
             int count = stats.deletedDocGCCount - stats.deletedLeafDocGCCount;
@@ -254,7 +255,7 @@ public class VersionGCRecommendations {
     }
 
     private void setLongSetting(String propName, long val) {
-        setLongSetting(Map.of(propName, val));
+        setLongSetting(of(propName, val));
     }
 
     private void setLongSetting(final Map<String, Long> propValMap) {
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
index 3f9cb23f9a..b562831e24 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
@@ -62,6 +62,7 @@ import static java.util.Collections.emptySet;
 import static java.util.Objects.requireNonNull;
 import static java.util.Optional.ofNullable;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.stream.Collectors.joining;
 import static org.apache.jackrabbit.guava.common.base.StandardSystemProperty.LINE_SEPARATOR;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.all;
 import static org.apache.jackrabbit.guava.common.collect.Iterators.partition;
@@ -604,7 +605,7 @@ public class VersionGarbageCollector {
          * @param headRevision the current head revision of node store
          */
         private void collectDetailedGarbage(final GCPhases phases, final RevisionVector headRevision, final VersionGCRecommendations rec)
-                throws IOException, LimitExceededException {
+                throws IOException {
             int docsTraversed = 0;
             boolean foundDoc = true;
             long oldestModifiedGCed = rec.scopeFullGC.fromMs;
@@ -647,11 +648,9 @@ public class VersionGarbageCollector {
                                     oldestModifiedGCed = modified;
                                 }
 
-                                if (gc.hasGarbage()) {
-                                    if (phases.start(GCPhase.DETAILED_GC_CLEANUP)) {
-                                        gc.removeGarbage(phases.stats);
-                                        phases.stop(GCPhase.DETAILED_GC_CLEANUP);
-                                    }
+                                if (gc.hasGarbage() && phases.start(GCPhase.DETAILED_GC_CLEANUP)) {
+                                    gc.removeGarbage(phases.stats);
+                                    phases.stop(GCPhase.DETAILED_GC_CLEANUP);
                                 }
 
                                 oldestModifiedGCed = modified == null ? fromModified : modified;
@@ -861,8 +860,8 @@ public class VersionGarbageCollector {
             monitor.info("Proceeding to update [{}] documents", updateOpList.size());
 
             if (log.isDebugEnabled()) {
-                String collect = updateOpList.stream().map(UpdateOp::getId).collect(Collectors.joining(","));
-                log.trace("Performing batch update of documents with following id's. \n" + collect);
+                String collect = updateOpList.stream().map(UpdateOp::getId).collect(joining(","));
+                log.debug("Performing batch update of documents with following id's [{}]", collect);
             }
 
             if (cancel.get()) {


[jackrabbit-oak] 04/08: OAK-10199 : override getModifiedDocs() for RDB and added unit cases for deletedProps

Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

daim pushed a commit to branch OAK-10199
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit 202dd8a9fbb55cd87a1a7dd6e93dd4d0b375e4d6
Author: Rishabh Kumar <di...@adobe.com>
AuthorDate: Wed Apr 26 21:16:44 2023 +0530

    OAK-10199 : override getModifiedDocs() for RDB and added unit cases for deletedProps
---
 .../oak/plugins/document/NodeDocument.java         |  12 +-
 .../plugins/document/VersionGCRecommendations.java |  36 +-
 .../plugins/document/VersionGarbageCollector.java  | 383 ++++++++++++---------
 .../document/mongo/MongoVersionGCSupport.java      |  51 ++-
 .../plugins/document/rdb/RDBDocumentStoreJDBC.java |   1 +
 .../plugins/document/rdb/RDBVersionGCSupport.java  |  56 +++
 .../oak/plugins/document/DetailGCHelper.java       |  22 +-
 .../oak/plugins/document/NodeDocumentTest.java     |  32 ++
 .../oak/plugins/document/VersionGCInitTest.java    |  17 +
 .../oak/plugins/document/VersionGCStatsTest.java   |  15 +
 .../oak/plugins/document/VersionGCTest.java        |  13 +-
 .../document/VersionGarbageCollectorIT.java        |  64 ++++
 12 files changed, 482 insertions(+), 220 deletions(-)

diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
index 71abba0a2e..66a1bc2eae 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
@@ -32,7 +32,6 @@ import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
-import java.util.stream.Collectors;
 
 import org.apache.jackrabbit.guava.common.base.Function;
 import org.apache.jackrabbit.guava.common.base.Predicate;
@@ -59,6 +58,7 @@ import org.apache.jackrabbit.guava.common.collect.Iterables;
 import org.apache.jackrabbit.guava.common.collect.Maps;
 import org.apache.jackrabbit.guava.common.collect.Sets;
 
+import static java.util.stream.Collectors.toSet;
 import static org.apache.jackrabbit.guava.common.base.Objects.equal;
 import static org.apache.jackrabbit.guava.common.base.Preconditions.checkArgument;
 import static org.apache.jackrabbit.guava.common.base.Preconditions.checkNotNull;
@@ -67,7 +67,6 @@ import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.mergeSorted;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.transform;
 import static java.util.Objects.requireNonNull;
-import static java.util.stream.Collectors.toMap;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
 import static org.apache.jackrabbit.oak.plugins.document.StableRevisionComparator.REVERSE;
 import static org.apache.jackrabbit.oak.plugins.document.UpdateOp.Key;
@@ -1672,17 +1671,16 @@ public final class NodeDocument extends Document {
     }
 
     /**
-     * Returns all the properties on this document
-     * @return Map of all properties along with their values
+     * Returns name of all the properties on this document
+     * @return Set of all property names
      */
     @NotNull
-    Map<String, SortedMap<Revision, String>> getProperties() {
+    Set<String> getPropertyNames() {
         return data
                 .keySet()
                 .stream()
                 .filter(Utils::isPropertyName)
-                .map(o -> Map.entry(o, getLocalMap(o)))
-                .collect(toMap(Entry::getKey, Entry::getValue));
+                .collect(toSet());
     }
 
     /**
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
index d8b091261d..f04b56fc52 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
@@ -18,6 +18,7 @@
  */
 package org.apache.jackrabbit.oak.plugins.document;
 
+import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
@@ -30,9 +31,7 @@ import org.apache.jackrabbit.oak.stats.Clock;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import org.apache.jackrabbit.guava.common.collect.Maps;
-
-import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP;
+import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP;
 import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP;
 
 /**
@@ -51,7 +50,7 @@ public class VersionGCRecommendations {
     final long maxCollect;
     final long deleteCandidateCount;
     final long lastOldestTimestamp;
-    final long fullDetailGCTimestamp;
+    final long detailedGCTimestamp;
     final long originalCollectLimit;
 
     private final long precisionMs;
@@ -96,7 +95,7 @@ public class VersionGCRecommendations {
         TimeInterval keep = new TimeInterval(clock.getTime() - maxRevisionAgeMs, Long.MAX_VALUE);
 
         Map<String, Long> settings = getLongSettings();
-        lastOldestTimestamp = settings.get(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP);
+        lastOldestTimestamp = settings.get(VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP);
         if (lastOldestTimestamp == 0) {
             log.debug("No lastOldestTimestamp found, querying for the oldest deletedOnce candidate");
             oldestPossible = vgc.getOldestDeletedOnceTimestamp(clock, options.precisionMs) - 1;
@@ -108,17 +107,17 @@ public class VersionGCRecommendations {
         TimeInterval scope = new TimeInterval(oldestPossible, Long.MAX_VALUE);
         scope = scope.notLaterThan(keep.fromMs);
 
-        fullDetailGCTimestamp = settings.get(SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP);
-        if (fullDetailGCTimestamp == 0) {
+        detailedGCTimestamp = settings.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP);
+        if (detailedGCTimestamp == 0) {
             if (log.isDebugEnabled()) {
-                log.debug("No fullDetailGCTimestamp found, querying for the oldest deletedOnce candidate");
+                log.debug("No detailedGCTimestamp found, querying for the oldest deletedOnce candidate");
             }
             oldestPossibleFullGC = vgc.getOldestModifiedTimestamp(clock) - 1;
             if (log.isDebugEnabled()) {
-                log.debug("fullDetailGCTimestamp found: {}", Utils.timestampToString(oldestPossibleFullGC));
+                log.debug("detailedGCTimestamp found: {}", Utils.timestampToString(oldestPossibleFullGC));
             }
         } else {
-            oldestPossibleFullGC = fullDetailGCTimestamp - 1;
+            oldestPossibleFullGC = detailedGCTimestamp - 1;
         }
 
         TimeInterval scopeFullGC = new TimeInterval(oldestPossibleFullGC, Long.MAX_VALUE);
@@ -206,10 +205,8 @@ public class VersionGCRecommendations {
             stats.needRepeat = true;
         } else if (!stats.canceled && !stats.ignoredGCDueToCheckPoint) {
             // success, we would not expect to encounter revisions older than this in the future
-//            setLongSetting(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs);
-//            setLongSetting(SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, stats.oldestModifiedGced);
             setLongSetting(ImmutableMap.of(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs,
-                    SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, stats.oldestModifiedGced));
+                    SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP, stats.oldestModifiedGced));
 
             int count = stats.deletedDocGCCount - stats.deletedLeafDocGCCount;
             double usedFraction;
@@ -240,11 +237,11 @@ public class VersionGCRecommendations {
 
     private Map<String, Long> getLongSettings() {
         Document versionGCDoc = vgc.getDocumentStore().find(Collection.SETTINGS, VersionGarbageCollector.SETTINGS_COLLECTION_ID, 0);
-        Map<String, Long> settings = Maps.newHashMap();
+        Map<String, Long> settings = new HashMap<>();
         // default values
-        settings.put(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, 0L);
+        settings.put(VersionGarbageCollector.SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, 0L);
         settings.put(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, 0L);
-        settings.put(SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, 0L);
+        settings.put(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP, 0L);
         if (versionGCDoc != null) {
             for (String k : versionGCDoc.keySet()) {
                 Object value = versionGCDoc.get(k);
@@ -256,14 +253,11 @@ public class VersionGCRecommendations {
         return settings;
     }
 
-    void setLongSetting(String propName, long val) {
+    private void setLongSetting(String propName, long val) {
         setLongSetting(Map.of(propName, val));
-//        UpdateOp updateOp = new UpdateOp(VersionGarbageCollector.SETTINGS_COLLECTION_ID, true);
-//        updateOp.set(propName, val);
-//        vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
     }
 
-    void setLongSetting(final Map<String, Long> propValMap) {
+    private void setLongSetting(final Map<String, Long> propValMap) {
         UpdateOp updateOp = new UpdateOp(VersionGarbageCollector.SETTINGS_COLLECTION_ID, true);
         propValMap.forEach(updateOp::set);
         vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
index 608ba02398..27ee36204a 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
@@ -21,16 +21,18 @@ package org.apache.jackrabbit.oak.plugins.document;
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
-import java.util.SortedMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Collectors;
 
 import org.apache.jackrabbit.guava.common.base.Function;
 import org.apache.jackrabbit.guava.common.base.Joiner;
@@ -55,7 +57,11 @@ import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static java.lang.Math.round;
+import static java.util.Collections.emptySet;
 import static java.util.Objects.requireNonNull;
+import static java.util.Optional.ofNullable;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.apache.jackrabbit.guava.common.base.StandardSystemProperty.LINE_SEPARATOR;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.all;
 import static org.apache.jackrabbit.guava.common.collect.Iterators.partition;
@@ -67,14 +73,10 @@ import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_I
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType.COMMIT_ROOT_ONLY;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType.DEFAULT_LEAF;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType.DEFAULT_NO_BRANCH;
-import static org.apache.jackrabbit.oak.plugins.document.StableRevisionComparator.REVERSE;
 import static org.slf4j.helpers.MessageFormatter.arrayFormat;
 
 public class VersionGarbageCollector {
 
-    /** TODO temporary global flag to enable 'detail gc' during prototyping. Should eventually become eg a system property */
-    public static boolean DETAIL_GC_ENABLED = false;
-
     //Kept less than MongoDocumentStore.IN_CLAUSE_BATCH_SIZE to avoid re-partitioning
     private static final int DELETE_BATCH_SIZE = 450;
     private static final int UPDATE_BATCH_SIZE = 450;
@@ -105,15 +107,9 @@ public class VersionGarbageCollector {
     static final String SETTINGS_COLLECTION_REC_INTERVAL_PROP = "recommendedIntervalMs";
 
     /**
-     * Property name to timestamp when last full-detail-GC run happened, or -1 if not applicable/in-use.
-     * <p>
-     * <ul>
-     * <li>-1 : full repo scan is disabled</li>
-     * <li>0 : full repo scan is enabled and bound to start from zero == oldest _modified </li>
-     * <li>gt 0 : full repo scan is enabled, was already done up until this value</li>
-     * </ul>
+     * Property name to timestamp till when last detailed-GC run happened
      */
-    static final String SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP = "fullDetailGCTimeStamp";
+    static final String SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP = "detailedGCTimeStamp";
 
     private final DocumentNodeStore nodeStore;
     private final DocumentStore ds;
@@ -264,7 +260,6 @@ public class VersionGarbageCollector {
     }
 
     public static class VersionGCStats {
-        public long oldestModifiedGced;
         boolean ignoredGCDueToCheckPoint;
         boolean canceled;
         boolean success = true;
@@ -276,18 +271,26 @@ public class VersionGarbageCollector {
         int splitDocGCCount;
         int intermediateSplitDocGCCount;
         int updateResurrectedGCCount;
+        long oldestModifiedGced;
+        int updatedDetailedGCDocsCount;
+        int deletedPropsGCCount;
         final TimeDurationFormatter df = TimeDurationFormatter.forLogging();
         final Stopwatch active = Stopwatch.createUnstarted();
         final Stopwatch collectDeletedDocs = Stopwatch.createUnstarted();
         final Stopwatch checkDeletedDocs = Stopwatch.createUnstarted();
-        final Stopwatch detailedGcDocs = Stopwatch.createUnstarted();
-        final Stopwatch deleteDeletedDocs = Stopwatch.createUnstarted();
-        final Stopwatch collectAndDeleteSplitDocs = Stopwatch.createUnstarted();
+        final Stopwatch detailedGCDocs = Stopwatch.createUnstarted();
+        final Stopwatch deleteDetailedGCDocs = Stopwatch.createUnstarted();
         final Stopwatch deleteSplitDocs = Stopwatch.createUnstarted();
         final Stopwatch sortDocIds = Stopwatch.createUnstarted();
         final Stopwatch updateResurrectedDocuments = Stopwatch.createUnstarted();
+        final Stopwatch deleteDeletedDocs = Stopwatch.createUnstarted();
+        final Stopwatch collectAndDeleteSplitDocs = Stopwatch.createUnstarted();
+        final Stopwatch collectDeletedProps = Stopwatch.createUnstarted();
+        final Stopwatch collectDeletedOldRevs = Stopwatch.createUnstarted();
+        final Stopwatch collectUnmergedBC = Stopwatch.createUnstarted();
         long activeElapsed, collectDeletedDocsElapsed, checkDeletedDocsElapsed, deleteDeletedDocsElapsed, collectAndDeleteSplitDocsElapsed,
-                deleteSplitDocsElapsed, sortDocIdsElapsed, updateResurrectedDocumentsElapsed, detailedGcDocsElapsed;
+                deleteSplitDocsElapsed, sortDocIdsElapsed, updateResurrectedDocumentsElapsed, detailedGCDocsElapsed, collectDeletedPropsElapsed,
+                deleteDetailedGCDocsElapsed, collectDeletedOldRevsElapsed, collectUnmergedBCElapsed;
 
         @Override
         public String toString() {
@@ -306,6 +309,11 @@ public class VersionGarbageCollector {
                         df.format(updateResurrectedDocumentsElapsed, MICROSECONDS),
                         df.format(deleteDeletedDocsElapsed, MICROSECONDS),
                         df.format(collectAndDeleteSplitDocsElapsed, MICROSECONDS),
+                        df.format(detailedGCDocsElapsed, MICROSECONDS),
+                        df.format(deleteDetailedGCDocsElapsed, MICROSECONDS),
+                        df.format(collectDeletedPropsElapsed, MICROSECONDS),
+                        df.format(collectDeletedOldRevsElapsed, MICROSECONDS),
+                        df.format(collectUnmergedBCElapsed, MICROSECONDS),
                         timeDeletingSplitDocs);
             } else {
                 String timeDeletingSplitDocs = "";
@@ -319,17 +327,24 @@ public class VersionGarbageCollector {
                         df.format(updateResurrectedDocuments.elapsed(MICROSECONDS), MICROSECONDS),
                         df.format(deleteDeletedDocs.elapsed(MICROSECONDS), MICROSECONDS),
                         df.format(collectAndDeleteSplitDocs.elapsed(MICROSECONDS), MICROSECONDS),
+                        df.format(detailedGCDocs.elapsed(MICROSECONDS), MICROSECONDS),
+                        df.format(deleteDetailedGCDocs.elapsed(MICROSECONDS), MICROSECONDS),
+                        df.format(collectDeletedProps.elapsed(MICROSECONDS), MICROSECONDS),
+                        df.format(collectDeletedOldRevs.elapsed(MICROSECONDS), MICROSECONDS),
+                        df.format(collectUnmergedBC.elapsed(MICROSECONDS), MICROSECONDS),
                         timeDeletingSplitDocs);
             }
 
             return "VersionGCStats{" +
                     "ignoredGCDueToCheckPoint=" + ignoredGCDueToCheckPoint +
-                    ", oldestModifiedGced=" + oldestModifiedGced +
                     ", canceled=" + canceled +
                     ", deletedDocGCCount=" + deletedDocGCCount + " (of which leaf: " + deletedLeafDocGCCount + ")" +
                     ", updateResurrectedGCCount=" + updateResurrectedGCCount +
                     ", splitDocGCCount=" + splitDocGCCount +
                     ", intermediateSplitDocGCCount=" + intermediateSplitDocGCCount +
+                    ", oldestModifiedGced=" + oldestModifiedGced +
+                    ", updatedDetailedGCDocsCount=" + updatedDetailedGCDocsCount +
+                    ", deletedPropsGCCount=" + deletedPropsGCCount +
                     ", iterationCount=" + iterationCount +
                     ", timeActive=" + df.format(activeElapsed, MICROSECONDS) +
                     ", " + timings + "}";
@@ -338,7 +353,6 @@ public class VersionGarbageCollector {
         void addRun(VersionGCStats run) {
             ++iterationCount;
             this.ignoredGCDueToCheckPoint = run.ignoredGCDueToCheckPoint;
-            this.oldestModifiedGced = run.oldestModifiedGced;
             this.canceled = run.canceled;
             this.success = run.success;
             this.limitExceeded = run.limitExceeded;
@@ -348,6 +362,9 @@ public class VersionGarbageCollector {
             this.splitDocGCCount += run.splitDocGCCount;
             this.intermediateSplitDocGCCount += run.intermediateSplitDocGCCount;
             this.updateResurrectedGCCount += run.updateResurrectedGCCount;
+            this.oldestModifiedGced = run.oldestModifiedGced;
+            this.updatedDetailedGCDocsCount += run.updatedDetailedGCDocsCount;
+            this.deletedPropsGCCount += run.deletedPropsGCCount;
             if (run.iterationCount > 0) {
                 // run is cumulative with times in elapsed fields
                 this.activeElapsed += run.activeElapsed;
@@ -358,7 +375,11 @@ public class VersionGarbageCollector {
                 this.deleteSplitDocsElapsed += run.deleteSplitDocsElapsed;
                 this.sortDocIdsElapsed += run.sortDocIdsElapsed;
                 this.updateResurrectedDocumentsElapsed += run.updateResurrectedDocumentsElapsed;
-                this.detailedGcDocsElapsed += run.detailedGcDocsElapsed;
+                this.detailedGCDocsElapsed += run.detailedGCDocsElapsed;
+                this.deleteDetailedGCDocsElapsed += run.deleteDetailedGCDocsElapsed;
+                this.collectDeletedPropsElapsed += run.collectDeletedPropsElapsed;
+                this.collectDeletedOldRevsElapsed += run.collectDeletedOldRevsElapsed;
+                this.collectUnmergedBCElapsed += run.collectUnmergedBCElapsed;
             } else {
                 // single run -> read from stop watches
                 this.activeElapsed += run.active.elapsed(MICROSECONDS);
@@ -369,7 +390,11 @@ public class VersionGarbageCollector {
                 this.deleteSplitDocsElapsed += run.deleteSplitDocs.elapsed(MICROSECONDS);
                 this.sortDocIdsElapsed += run.sortDocIds.elapsed(MICROSECONDS);
                 this.updateResurrectedDocumentsElapsed += run.updateResurrectedDocuments.elapsed(MICROSECONDS);
-                this.detailedGcDocsElapsed += run.detailedGcDocs.elapsed(MICROSECONDS);
+                this.detailedGCDocsElapsed += run.detailedGCDocs.elapsed(MICROSECONDS);
+                this.deleteDetailedGCDocsElapsed += run.deleteDetailedGCDocs.elapsed(MICROSECONDS);
+                this.collectDeletedPropsElapsed += run.collectDeletedProps.elapsed(MICROSECONDS);
+                this.collectDeletedOldRevsElapsed += run.collectDeletedOldRevs.elapsed(MICROSECONDS);
+                this.collectUnmergedBCElapsed += run.collectUnmergedBC.elapsed(MICROSECONDS);
             }
         }
     }
@@ -378,10 +403,14 @@ public class VersionGarbageCollector {
         NONE,
         COLLECTING,
         CHECKING,
-        DETAILED_GC,
         DELETING,
         SORTING,
         SPLITS_CLEANUP,
+        DETAILED_GC,
+        COLLECT_PROPS,
+        COLLECT_OLD_REVS,
+        COLLECT_UNMERGED_BC,
+        DETAILED_GC_CLEANUP,
         UPDATING
     }
 
@@ -406,11 +435,15 @@ public class VersionGarbageCollector {
             this.watches.put(GCPhase.NONE, Stopwatch.createStarted());
             this.watches.put(GCPhase.COLLECTING, stats.collectDeletedDocs);
             this.watches.put(GCPhase.CHECKING, stats.checkDeletedDocs);
-            this.watches.put(GCPhase.DETAILED_GC, stats.detailedGcDocs);
             this.watches.put(GCPhase.DELETING, stats.deleteDeletedDocs);
             this.watches.put(GCPhase.SORTING, stats.sortDocIds);
             this.watches.put(GCPhase.SPLITS_CLEANUP, stats.collectAndDeleteSplitDocs);
             this.watches.put(GCPhase.UPDATING, stats.updateResurrectedDocuments);
+            this.watches.put(GCPhase.DETAILED_GC, stats.detailedGCDocs);
+            this.watches.put(GCPhase.COLLECT_PROPS, stats.collectDeletedProps);
+            this.watches.put(GCPhase.COLLECT_OLD_REVS, stats.collectDeletedOldRevs);
+            this.watches.put(GCPhase.COLLECT_UNMERGED_BC, stats.collectUnmergedBC);
+            this.watches.put(GCPhase.DETAILED_GC_CLEANUP, stats.deleteDetailedGCDocs);
             this.canceled = canceled;
         }
 
@@ -534,7 +567,7 @@ public class VersionGarbageCollector {
                     collectDeletedDocuments(phases, headRevision, rec);
                     collectSplitDocuments(phases, sweepRevisions, rec);
                     if (detailedGCEnabled) {
-                        // run only if enabled
+                        // run only if detailed GC enabled
                         collectDetailedGarbage(phases, headRevision, rec);
                     }
                 }
@@ -568,102 +601,74 @@ public class VersionGarbageCollector {
          * it is okay that it takes a considerable amount of time.
          *
          * @param phases {@link GCPhases}
-         * @param headRevision the current head revision of
-         * @throws IOException
-         * @throws LimitExceededException
+         * @param headRevision the current head revision of node store
          */
         private void collectDetailedGarbage(final GCPhases phases, final RevisionVector headRevision, final VersionGCRecommendations rec)
                 throws IOException, LimitExceededException {
             int docsTraversed = 0;
-            long oldestModifiedGced = rec.scopeFullGC.fromMs;
+            boolean foundDoc = true;
+            long oldestModifiedGCed = rec.scopeFullGC.fromMs;
             try (DetailedGC gc = new DetailedGC(headRevision, monitor, cancel)) {
                 final long fromModified = rec.scopeFullGC.fromMs;
                 final long toModified = rec.scopeFullGC.toMs;
-//                if (rec.fullDetailGCTimestamp == -1) {
-//                    // then full detail-gc is disabled or over - use regular scope then
-//                    fromModified = rec.scope.fromMs;
-//                    toModified = rec.scope.toMs;
-//                } else {
-//                    // then full detail-gc is enabled - use it then
-//                    fromModified = rec.fullDetailGCTimestamp; // TODO: once we're passed rec.scope.fromMs we should
-//                    // disable fullgc
-//                    toModified = rec.scope.toMs; // the 'to' here is the max. it will process only eg 1 batch
-//                }
-                // TODO : remove me
-                boolean foundAnything = false; // I think this flag is redundant
-                if (phases.start(GCPhase.COLLECTING)) {
-                    Iterable<NodeDocument> itr = versionStore.getModifiedDocs(oldestModifiedGced, toModified, 2000);
-                    final Stopwatch timer = Stopwatch.createUnstarted();
-                    timer.reset().start();
-                    try {
-                        for (NodeDocument doc : itr) {
-                            // continue with GC?
-                            if (cancel.get()) {
-                                break;
-                            }
-                            if (phases.start(GCPhase.DETAILED_GC)) {
-                                gc.detailedGC(doc, phases);
-                                phases.stop(GCPhase.DETAILED_GC);
-                            }
+                if (phases.start(GCPhase.DETAILED_GC)) {
+                    while (foundDoc && oldestModifiedGCed < toModified && docsTraversed <= PROGRESS_BATCH_SIZE) {
+                        // set foundDoc to false to allow exiting the while loop
+                        foundDoc = false;
+                        Iterable<NodeDocument> itr = versionStore.getModifiedDocs(oldestModifiedGCed, toModified, 1000);
+                        try {
+                            for (NodeDocument doc : itr) {
+                                foundDoc = true;
+                                // continue with GC?
+                                if (cancel.get()) {
+                                    break;
+                                }
+                                docsTraversed++;
+                                if (docsTraversed % PROGRESS_BATCH_SIZE == 0) {
+                                    monitor.info("Iterated through {} documents so far. {} had detail garbage",
+                                            docsTraversed, gc.getGarbageDocsCount());
+                                }
 
-                            // TODO : remove this code, I don't think its possible to fetch these documents
-                            //  who doesn't have _modified field
-                            final Long modified = doc.getModified();
-                            if (modified == null) {
-                                monitor.warn("collectDetailGarbage : document has no _modified property : {}",
-                                        doc.getId());
-                            } else if (modified < oldestModifiedGced) {
-                                monitor.warn(
-                                        "collectDetailGarbage : document has older _modified than query boundary : {} (from: {}, to: {})",
-                                        modified, fromModified, toModified);
-                            } else {
-                                oldestModifiedGced = modified;
-                            }
-                            foundAnything = true;
-                            docsTraversed++;
-                            if (docsTraversed % PROGRESS_BATCH_SIZE == 0) {
-                                monitor.info("Iterated through {} documents so far. {} had detail garbage",
-                                        docsTraversed, gc.getNumDocuments());
-                            }
-                            // this would never hit, since we are only fetching the oldest 2000 element in batches of 1000
-                            // TODO: remove this if above mentioned logic is fine
-                            if (rec.maxCollect > 0 && gc.getNumDocuments() > rec.maxCollect) {
-                                // TODO: how would we recover from this?
-                                // If we don't want above solution, then one of the another solution is to use lower time duration
-                                // as done in document deletion process or use lower limit value or
-                                // we should perform all the update ops in 1 go
-                                throw new LimitExceededException();
+                                // collect the data to delete in next step
+                                if (phases.start(GCPhase.COLLECTING)) {
+                                    gc.collectGarbage(doc, phases);
+                                    phases.stop(GCPhase.COLLECTING);
+                                }
+
+                                // TODO : remove this code, I don't think its possible to fetch these documents
+                                //  who doesn't have _modified field
+                                final Long modified = doc.getModified();
+                                if (modified == null) {
+                                    monitor.warn("collectDetailGarbage : document has no _modified property : {}",
+                                            doc.getId());
+                                } else if (modified < oldestModifiedGCed) {
+                                    monitor.warn(
+                                            "collectDetailGarbage : document has older _modified than query boundary : {} (from: {}, to: {})",
+                                            modified, fromModified, toModified);
+                                } else {
+                                    oldestModifiedGCed = modified;
+                                }
+
+                                if (gc.hasGarbage()) {
+                                    if (phases.start(GCPhase.DETAILED_GC_CLEANUP)) {
+                                        gc.removeGarbage(phases.stats);
+                                        phases.stop(GCPhase.DETAILED_GC_CLEANUP);
+                                    }
+                                }
+
+                                oldestModifiedGCed = modified == null ? fromModified : modified;
                             }
-                            oldestModifiedGced = modified == null ? fromModified : modified;
+                        } finally {
+                            Utils.closeIfCloseable(itr);
+                            phases.stats.oldestModifiedGced = oldestModifiedGCed;
                         }
-                    } finally {
-                        Utils.closeIfCloseable(itr);
-                        // why do we need to stop this here, we are already stopping the original gc run.
-                        // can this be removed
-                        delayOnModifications(timer.stop().elapsed(TimeUnit.MILLISECONDS));
-                        phases.stats.oldestModifiedGced = oldestModifiedGced;
                     }
-                    phases.stop(GCPhase.COLLECTING);
-//                    if (!cancel.get() && foundAnything) {
-//                        // TODO: move to evaluate()
-//                        rec.setLongSetting(SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, oldestModifiedGced + 1);
-//                    }
+                    phases.stop(GCPhase.DETAILED_GC);
                 }
             }
         }
 
-        private void delayOnModifications(long durationMs) {
-            long delayMs = Math.round(durationMs * options.delayFactor);
-            if (!cancel.get() && delayMs > 0) {
-                try {
-                    Clock clock = nodeStore.getClock();
-                    clock.waitUntil(clock.getTime() + delayMs);
-                }
-                catch (InterruptedException ex) {
-                    /* ignore */
-                }
-            }
-        }
+
 
         private void collectSplitDocuments(GCPhases phases,
                                            RevisionVector sweepRevisions,
@@ -752,77 +757,146 @@ public class VersionGarbageCollector {
         }
     }
 
-    private static class DetailedGC implements Closeable {
+    private class DetailedGC implements Closeable {
 
         private final RevisionVector headRevision;
         private final GCMonitor monitor;
         private final AtomicBoolean cancel;
-        private int count;
+        private final Stopwatch timer;
+        private final List<UpdateOp> updateOpList;
+        private int garbageDocsCount;
 
         public DetailedGC(@NotNull RevisionVector headRevision, @NotNull GCMonitor monitor, @NotNull AtomicBoolean cancel) {
             this.headRevision = requireNonNull(headRevision);
             this.monitor = monitor;
             this.cancel = cancel;
+            this.updateOpList = new ArrayList<>();
+            this.timer = Stopwatch.createUnstarted();
         }
 
-        public void detailedGC(NodeDocument doc, GCPhases phases) {
-//            deleteSample(doc, phases);
-            UpdateOp updateOp = new UpdateOp(requireNonNull(doc.getId()), false);
-            deleteDeletedProperties(doc, phases, updateOp);
-            deleteUnmergedBranchCommitDocument(doc, phases, updateOp);
-            deleteOldRevisions(doc, phases, updateOp);
+        public void collectGarbage(final NodeDocument doc, final GCPhases phases) {
+
+            monitor.info("Collecting Detailed Garbage for doc [{}]", doc.getId());
+
+            final UpdateOp op = new UpdateOp(requireNonNull(doc.getId()), false);
+            collectDeletedProperties(doc, phases, op);
+            collectUnmergedBranchCommitDocument(doc, phases, op);
+            collectOldRevisions(doc, phases, op);
+            // only add if there are changes for this doc
+            if (op.hasChanges()) {
+                garbageDocsCount++;
+                monitor.info("Collected [{}] garbage for doc [{}]", op.getChanges().size(), doc.getId());
+                updateOpList.add(op);
+            }
         }
 
-        /** TODO remove, this is just a skeleton sample */
-//        private void deleteSample(NodeDocument doc, GCPhases phases) {
-//            if (doc.getId().contains("should_delete")) {
-//                if (phases.start(GCPhase.DELETING)) {
-//                    monitor.info("deleteSample: should do the deletion now, but this is demo only. I'm still learning");
-//                    System.out.println("do the actual deletion");
-//                    count++;
-//                    phases.stop(GCPhase.DELETING);
-//                }
-//            }
-//        }
+        private boolean hasGarbage() {
+            return garbageDocsCount > 0;
+        }
 
-        private void deleteUnmergedBranchCommitDocument(NodeDocument doc, GCPhases phases, UpdateOp updateOp) {
-            // TODO Auto-generated method stub
+        private void collectUnmergedBranchCommitDocument(final NodeDocument doc, final GCPhases phases, final UpdateOp updateOp) {
+            if (phases.start(GCPhase.COLLECT_UNMERGED_BC)){
+                // TODO add umerged BC collection logic
+                phases.stop(GCPhase.COLLECT_UNMERGED_BC);
+            }
 
         }
 
-        private void deleteDeletedProperties(final NodeDocument doc, final GCPhases phases, final UpdateOp updateOp) {
+        private void collectDeletedProperties(final NodeDocument doc, final GCPhases phases, final UpdateOp updateOp) {
 
             // get Map of all properties along with their values
-            final Map<String, SortedMap<Revision, String>> properties = doc.getProperties();
-
-            // find all the properties which can be removed from document
-            // All the properties whose value is null in their respective
-            // latest revision are eligible to be garbage collected.
-            properties.forEach((propName, revisionStringSortedMap) -> {
-                if (revisionStringSortedMap.keySet()
-                        .stream()
-                        .sorted(REVERSE)
-                        .limit(1)
-                        .anyMatch(revision -> revisionStringSortedMap.get(revision) == null)) {
-                    // set this property for removal
-                    updateOp.remove(propName);
+            if (phases.start(GCPhase.COLLECT_PROPS)) {
+                final Set<String> properties = doc.getPropertyNames();
+
+                // find all the properties which can be removed from document.
+                // All the properties whose value is null in head revision are
+                // eligible to be garbage collected.
+
+                final Set<String> retainPropSet = ofNullable(doc.getNodeAtRevision(nodeStore, headRevision, null))
+                        .map(DocumentNodeState::getPropertyNames)
+                        .orElse(emptySet());
+                final int deletedPropsGCCount = properties.stream()
+                        .filter(p -> !retainPropSet.contains(p))
+                        .mapToInt(x -> {
+                            updateOp.remove(x);
+                            return 1;})
+                        .sum();
+
+
+                phases.stats.deletedPropsGCCount += deletedPropsGCCount;
+                if (log.isDebugEnabled()) {
+                    log.debug("Collected {} deleted properties for document {}", deletedPropsGCCount, doc.getId());
                 }
-            });
+                phases.stop(GCPhase.COLLECT_PROPS);
+            }
         }
 
-        private void deleteOldRevisions(NodeDocument doc, GCPhases phases, UpdateOp updateOp) {
-            // TODO Auto-generated method stub
+        private void collectOldRevisions(NodeDocument doc, GCPhases phases, UpdateOp updateOp) {
+
+            if (phases.start(GCPhase.COLLECT_OLD_REVS)){
+                // TODO add old rev collection logic
+                phases.stop(GCPhase.COLLECT_OLD_REVS);
+            }
 
         }
 
-        long getNumDocuments() {
-            return count;
+        int getGarbageDocsCount() {
+            return garbageDocsCount;
         }
 
         @Override
         public void close() throws IOException {
 
         }
+
+        public void removeGarbage(final VersionGCStats stats) {
+
+            if (updateOpList.isEmpty()) {
+                if (log.isDebugEnabled()) {
+                    log.debug("Skipping removal of detailed garbage, cause no garbage detected");
+                }
+                return;
+            }
+
+            int updatedDocs;
+
+            monitor.info("Proceeding to update [{}] documents", updateOpList.size());
+
+            if (log.isDebugEnabled()) {
+                String collect = updateOpList.stream().map(UpdateOp::getId).collect(Collectors.joining(","));
+                log.trace("Performing batch update of documents with following id's. \n" + collect);
+            }
+
+            if (cancel.get()) {
+                log.info("Aborting the removal of detailed garbage since RGC had been cancelled");
+                return;
+            }
+
+            timer.reset().start();
+            try {
+                // TODO create an api to bulk update findAndUpdate Ops
+                updatedDocs = (int) updateOpList.stream().map(op -> ds.findAndUpdate(NODES, op)).filter(Objects::nonNull).count();
+                stats.updatedDetailedGCDocsCount += updatedDocs;
+                log.info("Updated [{}] documents", updatedDocs);
+                // now reset delete metadata
+                updateOpList.clear();
+                garbageDocsCount = 0;
+            } finally {
+                delayOnModifications(timer.stop().elapsed(MILLISECONDS), cancel);
+            }
+        }
+    }
+    private void delayOnModifications(final long durationMs, final AtomicBoolean cancel) {
+        long delayMs = round(durationMs * options.delayFactor);
+        if (!cancel.get() && delayMs > 0) {
+            try {
+                Clock clock = nodeStore.getClock();
+                clock.waitUntil(clock.getTime() + delayMs);
+            }
+            catch (InterruptedException ex) {
+                /* ignore */
+            }
+        }
     }
 
     /**
@@ -959,19 +1033,6 @@ public class VersionGarbageCollector {
 
         //------------------------------< internal >----------------------------
 
-        private void delayOnModifications(long durationMs) {
-            long delayMs = Math.round(durationMs * options.delayFactor);
-            if (!cancel.get() && delayMs > 0) {
-                try {
-                    Clock clock = nodeStore.getClock();
-                    clock.waitUntil(clock.getTime() + delayMs);
-                }
-                catch (InterruptedException ex) {
-                    /* ignore */
-                }
-            }
-        }
-
         private Iterator<String> previousDocIdsFor(NodeDocument doc) {
             Map<Revision, Range> prevRanges = doc.getPreviousRanges(true);
             if (prevRanges.isEmpty()) {
@@ -1127,7 +1188,7 @@ public class VersionGarbageCollector {
                         monitor.info(msg);
                     }
                 } finally {
-                    delayOnModifications(timer.stop().elapsed(TimeUnit.MILLISECONDS));
+                    delayOnModifications(timer.stop().elapsed(TimeUnit.MILLISECONDS), cancel);
                 }
             }
             return deletedCount;
@@ -1160,7 +1221,7 @@ public class VersionGarbageCollector {
                 }
             }
             finally {
-                delayOnModifications(timer.stop().elapsed(TimeUnit.MILLISECONDS));
+                delayOnModifications(timer.stop().elapsed(TimeUnit.MILLISECONDS), cancel);
             }
             return updateCount;
         }
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
index e34d8f36b0..4d01e5d3da 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
@@ -19,6 +19,8 @@
 
 package org.apache.jackrabbit.oak.plugins.document.mongo;
 
+import static java.util.Optional.ofNullable;
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.concat;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.transform;
@@ -42,6 +44,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
 import java.util.regex.Pattern;
 
 import org.apache.jackrabbit.oak.plugins.document.Document;
@@ -111,10 +114,10 @@ public class MongoVersionGCSupport extends VersionGCSupport {
     @Override
     public CloseableIterable<NodeDocument> getPossiblyDeletedDocs(final long fromModified, final long toModified) {
         //_deletedOnce == true && _modified >= fromModified && _modified < toModified
-        Bson query = and(
+        Bson query = Filters.and(
                 Filters.eq(DELETED_ONCE, true),
-                gte(MODIFIED_IN_SECS, getModifiedInSecs(fromModified)),
-                lt(MODIFIED_IN_SECS, getModifiedInSecs(toModified))
+                Filters.gte(MODIFIED_IN_SECS, getModifiedInSecs(fromModified)),
+                Filters.lt(MODIFIED_IN_SECS, getModifiedInSecs(toModified))
         );
         FindIterable<BasicDBObject> cursor = getNodeCollection()
                 .find(query).batchSize(batchSize);
@@ -139,9 +142,10 @@ public class MongoVersionGCSupport extends VersionGCSupport {
         // _modified >= fromModified && _modified < toModified
         final Bson query = and(gte(MODIFIED_IN_SECS, getModifiedInSecs(fromModified)),
                 lt(MODIFIED_IN_SECS, getModifiedInSecs(toModified)));
+        final Bson sort = Filters.eq(MODIFIED_IN_SECS, 1);
         final FindIterable<BasicDBObject> cursor = getNodeCollection()
                 .find(query)
-                .sort(new org.bson.Document(MODIFIED_IN_SECS, 1))
+                .sort(sort)
                 .limit(limit);
         return CloseableIterable.wrap(transform(cursor, input -> store.convertFromDBObject(NODES, input)));
     }
@@ -219,6 +223,35 @@ public class MongoVersionGCSupport extends VersionGCSupport {
         return result.get(0);
     }
 
+    /**
+     * Retrieve the time of the oldest modified document.
+     *
+     * @param clock System Clock to measure time in accuracy of millis
+     * @return the timestamp of the oldest modified document.
+     */
+    @Override
+    public long getOldestModifiedTimestamp(final Clock clock) {
+        LOG.info("getOldestModifiedTimestamp() <- start");
+
+        final Bson sort = Filters.eq(MODIFIED_IN_SECS, 1);
+        final List<Long> result = new ArrayList<>(1);
+
+        getNodeCollection().find().sort(sort).limit(1).forEach(
+                (Consumer<BasicDBObject>) document ->
+                        ofNullable(store.convertFromDBObject(NODES, document))
+                                .ifPresent(doc -> {
+                    long modifiedMs = SECONDS.toMillis(ofNullable(doc.getModified()).orElse(0L));
+                    LOG.info("getOldestDeletedOnceTimestamp() -> {}", Utils.timestampToString(modifiedMs));
+                    result.add(modifiedMs);
+                }));
+
+        if (result.isEmpty()) {
+            LOG.info("getOldestModifiedTimestamp() -> none found, return current time");
+            result.add(clock.getTime());
+        }
+        return result.get(0);
+    }
+
     private List<Bson> createQueries(Set<SplitDocType> gcTypes,
                                  RevisionVector sweepRevs,
                                  long oldestRevTimeStamp) {
@@ -233,9 +266,9 @@ public class MongoVersionGCSupport extends VersionGCSupport {
         }
         // OAK-8351: this (last) query only contains SD_TYPE and SD_MAX_REV_TIME_IN_SECS
         // so mongodb should really use that _sdType_1__sdMaxRevTime_1 index
-        result.add(and(
+        result.add(Filters.and(
                 Filters.or(orClauses),
-                lt(SD_MAX_REV_TIME_IN_SECS, getModifiedInSecs(oldestRevTimeStamp))
+                Filters.lt(SD_MAX_REV_TIME_IN_SECS, getModifiedInSecs(oldestRevTimeStamp))
                 ));
 
         return result;
@@ -266,16 +299,16 @@ public class MongoVersionGCSupport extends VersionGCSupport {
             Bson idPathClause = Filters.or(
                     Filters.regex(ID, Pattern.compile(".*" + idSuffix)),
                     // previous documents with long paths do not have a '-' in the id
-                    and(
+                    Filters.and(
                             Filters.regex(ID, Pattern.compile("[^-]*")),
                             Filters.regex(PATH, Pattern.compile(".*" + idSuffix))
                     )
             );
 
             long minMaxRevTimeInSecs = Math.min(maxRevTimeInSecs, getModifiedInSecs(r.getTimestamp()));
-            result.add(and(
+            result.add(Filters.and(
                     Filters.eq(SD_TYPE, DEFAULT_NO_BRANCH.typeCode()),
-                    lt(SD_MAX_REV_TIME_IN_SECS, minMaxRevTimeInSecs),
+                    Filters.lt(SD_MAX_REV_TIME_IN_SECS, minMaxRevTimeInSecs),
                     idPathClause
                     ));
         }
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStoreJDBC.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStoreJDBC.java
index 5caa65d875..26fc1311fa 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStoreJDBC.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStoreJDBC.java
@@ -715,6 +715,7 @@ public class RDBDocumentStoreJDBC {
         }
 
         if (sortBy != null) {
+            // FIXME : order should be determined via sortBy field
             query.append(" order by ID");
         }
 
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java
index a463499793..f26268bcd3 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBVersionGCSupport.java
@@ -16,7 +16,13 @@
  */
 package org.apache.jackrabbit.oak.plugins.document.rdb;
 
+import static java.util.Collections.emptyList;
+import static java.util.List.of;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
+import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_IN_SECS;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.getModifiedInSecs;
+import static org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.EMPTY_KEY_PATTERN;
 
 import java.io.Closeable;
 import java.io.IOException;
@@ -85,6 +91,29 @@ public class RDBVersionGCSupport extends VersionGCSupport {
         }
     }
 
+    /**
+     * Returns documents that have a {@link NodeDocument#MODIFIED_IN_SECS} value
+     * within the given range .The two passed modified timestamps are in milliseconds
+     * since the epoch and the implementation will convert them to seconds at
+     * the granularity of the {@link NodeDocument#MODIFIED_IN_SECS} field and
+     * then perform the comparison.
+     *
+     * @param fromModified the lower bound modified timestamp (inclusive)
+     * @param toModified   the upper bound modified timestamp (exclusive)
+     * @param limit        the limit of documents to return
+     * @return matching documents.
+     */
+    @Override
+    public Iterable<NodeDocument> getModifiedDocs(final long fromModified, final long toModified, final int limit) {
+        List<QueryCondition> conditions = of(new QueryCondition(MODIFIED_IN_SECS, "<", getModifiedInSecs(toModified)),
+                new QueryCondition(MODIFIED_IN_SECS, ">=", getModifiedInSecs(fromModified)));
+        if (MODE == 1) {
+            return getIterator(EMPTY_KEY_PATTERN, conditions);
+        } else {
+            return store.queryAsIterable(NODES, null, null, EMPTY_KEY_PATTERN, conditions, limit, MODIFIED_IN_SECS);
+        }
+    }
+
     @Override
     protected Iterable<NodeDocument> identifyGarbage(final Set<SplitDocType> gcTypes, final RevisionVector sweepRevs,
             final long oldestRevTimeStamp) {
@@ -239,6 +268,33 @@ public class RDBVersionGCSupport extends VersionGCSupport {
         }
     }
 
+    /**
+     * Retrieve the time of the oldest modified document.
+     *
+     * @param clock System Clock
+     * @return the timestamp of the oldest modified document.
+     */
+    @Override
+    public long getOldestModifiedTimestamp(Clock clock) {
+        long modifiedMs = Long.MIN_VALUE;
+
+        LOG.info("getOldestModifiedTimestamp() <- start");
+        try {
+            long modifiedSec = store.getMinValue(NODES, MODIFIED_IN_SECS, null, null, EMPTY_KEY_PATTERN, emptyList());
+            modifiedMs = TimeUnit.SECONDS.toMillis(modifiedSec);
+        } catch (DocumentStoreException ex) {
+            LOG.error("getOldestModifiedTimestamp()", ex);
+        }
+
+        if (modifiedMs > 0) {
+            LOG.info("getOldestModifiedTimestamp() -> {}", Utils.timestampToString(modifiedMs));
+            return modifiedMs;
+        } else {
+            LOG.info("getOldestModifiedTimestamp() -> none found, return current time");
+            return clock.getTime();
+        }
+    }
+
     @Override
     public long getDeletedOnceCount() {
         return store.queryCount(Collection.NODES, null, null, RDBDocumentStore.EMPTY_KEY_PATTERN,
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DetailGCHelper.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DetailGCHelper.java
index 8a585c7dc0..d52c5e33ae 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DetailGCHelper.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DetailGCHelper.java
@@ -18,25 +18,19 @@
  */
 package org.apache.jackrabbit.oak.plugins.document;
 
-public class DetailGCHelper {
+import static org.apache.commons.lang3.reflect.FieldUtils.writeField;
 
-    public static void setLongSetting(String propName, long val, DocumentNodeStore ns) {
-        UpdateOp updateOp = new UpdateOp(VersionGarbageCollector.SETTINGS_COLLECTION_ID, true);
-        updateOp.set(propName, val);
-        ns.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
-    }
+public class DetailGCHelper {
 
-    public static void enableDetailGC(DocumentNodeStore ns) {
-        VersionGarbageCollector.DETAIL_GC_ENABLED = true;
-        if (ns != null) {
-            setLongSetting(VersionGarbageCollector.SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, 0, ns);
+    public static void enableDetailGC(final VersionGarbageCollector vgc) throws IllegalAccessException {
+        if (vgc != null) {
+            writeField(vgc, "detailedGCEnabled", true, true);
         }
     }
 
-    public static void disableDetailGC(DocumentNodeStore ns) {
-        VersionGarbageCollector.DETAIL_GC_ENABLED = false;
-        if (ns != null) {
-            setLongSetting(VersionGarbageCollector.SETTINGS_COLLECTION_FULL_DETAILGC_TIMESTAMP_PROP, -1, ns);
+    public static void disableDetailGC(final VersionGarbageCollector vgc) throws IllegalAccessException {
+        if (vgc != null) {
+            writeField(vgc, "detailedGCEnabled", false, true);
         }
     }
 }
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
index b99897dfb0..2adcc9f86a 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
@@ -449,6 +449,38 @@ public class NodeDocumentTest {
         assertEquals(140, uncommittedRevisions);
     }
 
+    @Test
+    public void getPropertyNames() throws CommitFailedException {
+        DocumentStore store = new MemoryDocumentStore();
+        DocumentNodeStore ns = new DocumentMK.Builder().setDocumentStore(store).setAsyncDelay(0).getNodeStore();
+
+        // add properties
+        for (int i = 0; i < 10; i++) {
+            NodeBuilder nb = ns.getRoot().builder();
+            nb.child("x").setProperty("p"+i, i);
+            merge(ns, nb);
+        }
+
+        final NodeDocument nodeDocument = store.find(NODES, "1:/x");
+        assert nodeDocument != null;
+        assertEquals(10, nodeDocument.getPropertyNames().size());
+    }
+
+    @Test
+    public void getNoPropertyNames() throws CommitFailedException {
+        DocumentStore store = new MemoryDocumentStore();
+        DocumentNodeStore ns = new DocumentMK.Builder().setDocumentStore(store).setAsyncDelay(0).getNodeStore();
+
+        // add no property
+        NodeBuilder nb = ns.getRoot().builder();
+        nb.child("x");
+        merge(ns, nb);
+
+        final NodeDocument nodeDocument = store.find(NODES, "1:/x");
+        assert nodeDocument != null;
+        assertEquals(0, nodeDocument.getPropertyNames().size());
+    }
+
     @Test
     public void getNewestRevisionTooExpensive() throws Exception {
         final int NUM_CHANGES = 200;
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java
index 055188be46..ed39a372b2 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java
@@ -22,6 +22,9 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 
+import static org.apache.jackrabbit.oak.plugins.document.DetailGCHelper.enableDetailGC;
+import static org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
@@ -49,6 +52,20 @@ public class VersionGCInitTest {
 
         vgc = store.find(Collection.SETTINGS, "versionGC");
         assertNotNull(vgc);
+        assertEquals(0L, vgc.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP));
     }
 
+    @Test
+    public void lazyInitializeWithDetailedGC() throws Exception {
+        DocumentStore store = ns.getDocumentStore();
+        Document vgc = store.find(Collection.SETTINGS, "versionGC");
+        assertNull(vgc);
+
+        enableDetailGC(ns.getVersionGarbageCollector());
+        ns.getVersionGarbageCollector().gc(1, TimeUnit.DAYS);
+
+        vgc = store.find(Collection.SETTINGS, "versionGC");
+        assertNotNull(vgc);
+        assertEquals(-1L, vgc.get(SETTINGS_COLLECTION_DETAILED_GC_TIMESTAMP_PROP));
+    }
 }
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCStatsTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCStatsTest.java
index 13448e5403..1515ea9312 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCStatsTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCStatsTest.java
@@ -67,6 +67,11 @@ public class VersionGCStatsTest {
         assertEquals(stats.collectAndDeleteSplitDocs.elapsed(MICROSECONDS), cumulative.collectAndDeleteSplitDocsElapsed);
         assertEquals(stats.sortDocIds.elapsed(MICROSECONDS), cumulative.sortDocIdsElapsed);
         assertEquals(stats.updateResurrectedDocuments.elapsed(MICROSECONDS), cumulative.updateResurrectedDocumentsElapsed);
+        assertEquals(stats.detailedGCDocs.elapsed(MICROSECONDS), cumulative.detailedGCDocsElapsed);
+        assertEquals(stats.deleteDetailedGCDocs.elapsed(MICROSECONDS), cumulative.deleteDetailedGCDocsElapsed);
+        assertEquals(stats.collectDeletedProps.elapsed(MICROSECONDS), cumulative.collectDeletedPropsElapsed);
+        assertEquals(stats.collectDeletedOldRevs.elapsed(MICROSECONDS), cumulative.collectDeletedOldRevsElapsed);
+        assertEquals(stats.collectUnmergedBC.elapsed(MICROSECONDS), cumulative.collectUnmergedBCElapsed);
     }
 
     @Test
@@ -83,6 +88,11 @@ public class VersionGCStatsTest {
         assertEquals(stats.collectAndDeleteSplitDocs.elapsed(MICROSECONDS) * 2, cumulative.collectAndDeleteSplitDocsElapsed);
         assertEquals(stats.sortDocIds.elapsed(MICROSECONDS) * 2, cumulative.sortDocIdsElapsed);
         assertEquals(stats.updateResurrectedDocuments.elapsed(MICROSECONDS) * 2, cumulative.updateResurrectedDocumentsElapsed);
+        assertEquals(stats.detailedGCDocs.elapsed(MICROSECONDS) * 2, cumulative.detailedGCDocsElapsed);
+        assertEquals(stats.deleteDetailedGCDocs.elapsed(MICROSECONDS) * 2, cumulative.deleteDetailedGCDocsElapsed);
+        assertEquals(stats.collectDeletedProps.elapsed(MICROSECONDS) * 2, cumulative.collectDeletedPropsElapsed);
+        assertEquals(stats.collectDeletedOldRevs.elapsed(MICROSECONDS) * 2, cumulative.collectDeletedOldRevsElapsed);
+        assertEquals(stats.collectUnmergedBC.elapsed(MICROSECONDS) * 2, cumulative.collectUnmergedBCElapsed);
     }
 
     private void forEachStopwatch(VersionGCStats stats, Callable c) {
@@ -93,6 +103,11 @@ public class VersionGCStatsTest {
         c.call(stats.collectAndDeleteSplitDocs);
         c.call(stats.sortDocIds);
         c.call(stats.updateResurrectedDocuments);
+        c.call(stats.detailedGCDocs);
+        c.call(stats.deleteDetailedGCDocs);
+        c.call(stats.collectDeletedProps);
+        c.call(stats.collectDeletedOldRevs);
+        c.call(stats.collectUnmergedBC);
     }
     
     private interface Callable {
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
index 48f3f362ce..f29716ca5d 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java
@@ -44,7 +44,6 @@ import org.jetbrains.annotations.NotNull;
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 
@@ -96,7 +95,7 @@ public class VersionGCTest {
 
     @After
     public void tearDown() throws Exception {
-        DetailGCHelper.disableDetailGC(ns);
+        DetailGCHelper.disableDetailGC(gc);
         execService.shutdown();
         execService.awaitTermination(1, MINUTES);
     }
@@ -343,21 +342,19 @@ public class VersionGCTest {
 
     // OAK-10199
     @Test
-    @Ignore
     public void testDetailGcDocumentRead_disabled() throws Exception {
-        DetailGCHelper.disableDetailGC(ns);
+        DetailGCHelper.disableDetailGC(gc);
         VersionGCStats stats = gc.gc(30, TimeUnit.MINUTES);
         assertNotNull(stats);
-        assertEquals(0, stats.detailedGcDocsElapsed);
+        assertEquals(0, stats.detailedGCDocsElapsed);
     }
 
     @Test
-    @Ignore
     public void testDetailGcDocumentRead_enabled() throws Exception {
-        DetailGCHelper.enableDetailGC(ns);
+        DetailGCHelper.enableDetailGC(gc);
         VersionGCStats stats = gc.gc(30, TimeUnit.MINUTES);
         assertNotNull(stats);
-        assertNotEquals(0, stats.detailedGcDocsElapsed);
+        assertNotEquals(0, stats.detailedGCDocsElapsed);
     }
 
     // OAK-10199
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
index d33cc8c7de..4470a07f96 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
@@ -34,10 +34,12 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
+import static org.apache.commons.lang3.reflect.FieldUtils.writeField;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.size;
 import static java.util.concurrent.TimeUnit.HOURS;
 import static java.util.concurrent.TimeUnit.MINUTES;
+import static org.apache.jackrabbit.oak.api.Type.STRING;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.NUM_REVS_THRESHOLD;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.PREV_SPLIT_FACTOR;
@@ -225,6 +227,68 @@ public class VersionGarbageCollectorIT {
     public void gcLongPathSplitDocs() throws Exception {
         gcSplitDocsInternal(Strings.repeat("sub", 120));
     }
+
+    @Test
+    public void testGCDeletedProps() throws Exception{
+        //1. Create nodes
+        NodeBuilder b1 = store.getRoot().builder();
+
+        // Add property to node & save
+        b1.child("x").setProperty("test", "t", STRING);
+        b1.child("z").setProperty("prop", "foo", STRING);
+        store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        // update the property
+        b1 = store.getRoot().builder();
+        b1.getChildNode("z").setProperty("prop", "bar", STRING);
+        store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        // update property again
+        b1 = store.getRoot().builder();
+        b1.getChildNode("z").setProperty("prop", "baz", STRING);
+        store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        // enable the detailed gc flag
+        writeField(gc, "detailedGCEnabled", true, true);
+        long maxAge = 1; //hours
+        long delta = TimeUnit.MINUTES.toMillis(10);
+        //1. Go past GC age and check no GC done as nothing deleted
+        clock.waitUntil(Revision.getCurrentTimestamp() + maxAge);
+        VersionGCStats stats = gc.gc(maxAge, HOURS);
+        assertEquals(0, stats.deletedPropsGCCount);
+
+        //Remove property
+        NodeBuilder b2 = store.getRoot().builder();
+        b2.getChildNode("z").removeProperty("prop");
+        store.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        store.runBackgroundOperations();
+
+        //2. Check that a deleted property is not collected before maxAge
+        //Clock cannot move back (it moved forward in #1) so double the maxAge
+        clock.waitUntil(clock.getTime() + delta);
+        stats = gc.gc(maxAge*2, HOURS);
+        assertEquals(0, stats.deletedPropsGCCount);
+
+        //3. Check that deleted property does get collected post maxAge
+        clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
+
+        stats = gc.gc(maxAge*2, HOURS);
+        assertEquals(1, stats.deletedPropsGCCount);
+
+        //4. Check that a revived property (deleted and created again) does not get gc
+        NodeBuilder b3 = store.getRoot().builder();
+        b3.child("x").removeProperty("test");
+        store.merge(b3, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        NodeBuilder b4 = store.getRoot().builder();
+        b4.child("x").setProperty("test", "t", STRING);
+        store.merge(b4, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);
+        stats = gc.gc(maxAge*2, HOURS);
+        assertEquals(0, stats.deletedPropsGCCount);
+    }
     
     private void gcSplitDocsInternal(String subNodeName) throws Exception {
         long maxAge = 1; //hrs


[jackrabbit-oak] 06/08: OAK-10199 : ignore documents which doesn't have _modified field in mongo while fetching modifiedDocs

Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

daim pushed a commit to branch OAK-10199
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit 1d47af5a8466db9506d45d5653bfc98e7086daa8
Author: Rishabh Kumar <di...@adobe.com>
AuthorDate: Tue May 30 20:02:13 2023 +0530

    OAK-10199 : ignore documents which doesn't have _modified field in mongo while fetching modifiedDocs
---
 .../oak/plugins/document/mongo/MongoVersionGCSupport.java     | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
index 4d01e5d3da..324ade704c 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java
@@ -19,6 +19,8 @@
 
 package org.apache.jackrabbit.oak.plugins.document.mongo;
 
+import static com.mongodb.client.model.Filters.eq;
+import static com.mongodb.client.model.Filters.exists;
 import static java.util.Optional.ofNullable;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.concat;
@@ -142,7 +144,7 @@ public class MongoVersionGCSupport extends VersionGCSupport {
         // _modified >= fromModified && _modified < toModified
         final Bson query = and(gte(MODIFIED_IN_SECS, getModifiedInSecs(fromModified)),
                 lt(MODIFIED_IN_SECS, getModifiedInSecs(toModified)));
-        final Bson sort = Filters.eq(MODIFIED_IN_SECS, 1);
+        final Bson sort = eq(MODIFIED_IN_SECS, 1);
         final FindIterable<BasicDBObject> cursor = getNodeCollection()
                 .find(query)
                 .sort(sort)
@@ -233,10 +235,13 @@ public class MongoVersionGCSupport extends VersionGCSupport {
     public long getOldestModifiedTimestamp(final Clock clock) {
         LOG.info("getOldestModifiedTimestamp() <- start");
 
-        final Bson sort = Filters.eq(MODIFIED_IN_SECS, 1);
+        final Bson sort = eq(MODIFIED_IN_SECS, 1);
         final List<Long> result = new ArrayList<>(1);
 
-        getNodeCollection().find().sort(sort).limit(1).forEach(
+        // we need to add query condition to ignore `previous` documents which doesn't have this field
+        final Bson query = exists(MODIFIED_IN_SECS);
+
+        getNodeCollection().find(query).sort(sort).limit(1).forEach(
                 (Consumer<BasicDBObject>) document ->
                         ofNullable(store.convertFromDBObject(NODES, document))
                                 .ifPresent(doc -> {