OAK-12010 Simplified index management (without optimizer) by thomasmueller · Pull Request #2689 · apache/jackrabbit-oak

thomasmueller · 2026-01-19T09:31:45Z

This is a subset of the (much) larger draft PR #2652

This is just support for the "diff.index"; without the optimizer.

bhabegger

Nice clean work 👍

I like the test coverage and the comments among the rest !

I don't see anything blocking and just have a few comments/suggestions.

bhabegger · 2026-01-30T13:58:39Z

+            if (!diffContent.exists()) {
+                continue;
+            }
+            PropertyState lastMod = diffContent.getProperty("jcr:lastModified");


Is there not a constant definition for this type of "well-known" entries ? (same for :lastProcessed, jcr:data, /oak:index/, etc.)

In my opinion this helps a lot to understand, identify in one go what are the 'system' properties, paths, etc., and know where they are used. But also have them documented in the code base as javadoc.

(Not a blocker, just a curiosity 😉)

There probably is. I agree we are not consistently using these constants, so there is a risk of typos.

I used constants now where available, and did add new ones where I think it is useful (which -- granted -- is subjective).

bhabegger · 2026-01-30T16:20:47Z

+import java.util.ArrayList;
+
+import org.apache.felix.inventory.Format;
+import org.apache.jackrabbit.oak.commons.json.JsonObject;


I'm guessing that OAK has it's own JSON processing library for historical reasons ?

bhabegger · 2026-01-30T16:22:59Z

+                + "    } }");
+
+        repositoryDefinitions = RootIndexesListService.getRootIndexDefinitions(store, "lucene");
+        assertSameJson("{\n"


It's a shame Java 11 is a bit too early for text blocks :)

I agree (btw. text blocks are not "raw" and still need escaping).

But I think it's readable as is.

bhabegger · 2026-01-30T16:26:22Z

+    public void createDummy() {
+        // when enabling "deleteCreatesDummyIndex", then a dummy index is created
+        // (that indexes /dummy, which doesn't exist)
+        String merged = new DiffIndexMerger(new String[0], true, true, false).processMerge(JsonObject.fromJson("{}"


Which true is it ?

Suggested change

String merged = new DiffIndexMerger(new String[0], true, true, false).processMerge(JsonObject.fromJson("{}"

boolean deleteCreatesDummyIndex = true;

String merged = new DiffIndexMerger(new String[0], deleteCreatesDummyIndex, true, false).processMerge(JsonObject.fromJson("{}"

Would make it explicit

Hm, "list of boolean flags" is not user friendly... thinking about an alternative.

I added setters (there no real harm to not have the fields final in this case). Sure we could also add a Builder. Or a Factory, or AdapterFactoryBuilder... :-)

…/diff/DiffIndex.java Co-authored-by: Benjamin Habegger <benjamin@habegger.tech>

amit-jain

Overall looks good to me!!
A few minor suggestions added.

amit-jain · 2026-02-03T05:23:54Z

+     *                                  (input)
+     * @return whether a new version of an index was added
+     */
+    public boolean processMerge(String indexName, JsonObject indexDiff, JsonObject newImageLuceneDefinitions, JsonObject combined) {


yes recommend breaking this method, example (from comments)

findLatestVersions

performMerge

isChanged

addMetadata

…/diff/DiffIndex.java Co-authored-by: Amit Jain <amitj@apache.org>

…/diff/DiffIndexMerger.java Co-authored-by: Amit Jain <amitj@apache.org>

…/diff/DiffIndex.java Co-authored-by: Amit Jain <amitj@apache.org>

…/diff/JsonNodeBuilder.java Co-authored-by: Benjamin Habegger <benjamin@habegger.tech>

…/diff/DiffIndex.java Co-authored-by: Amit Jain <amitj@apache.org>

sonarqubecloud · 2026-02-06T17:16:17Z

Quality Gate passed

Issues
57 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.3% Coverage on New Code
1.9% Duplication on New Code

See analysis details on SonarQube Cloud

ChlineSaurus

Overall looks good to me, 😌 good clean code
I added a few comments

ChlineSaurus · 2026-02-10T12:36:56Z

What's the purpose of the four files on the oak-core root level? For me it looks like duplicate code to code in /oak/plugins/diff and to the test code 🤔

Thanks a lot! I committed them by mistake, now removed.

ChlineSaurus · 2026-02-10T12:41:35Z

+        log("Latest customized: {}", latestCustomizedKey);
+        if (latestProduct == null) {
+            if (indexName.indexOf('.') >= 0) {
+                // a fully custom index needs to contains a dot


Detail: Typo, should be "contain"

ChlineSaurus · 2026-02-10T12:43:23Z

+            merged = processMerge(latestProductIndex, indexDiff);
+        }
+
+        // compare to the latest version of the this index


Detail: Typo "the this index" to "the index"

ChlineSaurus · 2026-02-10T12:47:00Z

+                }
+            }
+            // store now, so a change is only processed once
+            diffContent.setProperty(DiffIndexMerger.LAST_PROCESSED, modified);


In my understanding, we always (even in case of failure) set the LAST_PROCESSED property, as it is set before the processing, and in the case of failure will be committed when the error is committed. Thus we don't have a retry in case of failure. Do I understand this right? And is this intentional?

Retry wouldn't make sense: it would fail again in the same way.

* OAK-12010 Simplified index management (without optimizer) * OAK-12010 Simplified index management * OAK-12010 Simplified index management * OAK-12010 Simplified index management * Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/diff/DiffIndex.java Co-authored-by: Benjamin Habegger <benjamin@habegger.tech> * Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/diff/DiffIndex.java Co-authored-by: Benjamin Habegger <benjamin@habegger.tech> * Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/diff/DiffIndex.java Co-authored-by: Amit Jain <amitj@apache.org> * Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/diff/DiffIndexMerger.java Co-authored-by: Amit Jain <amitj@apache.org> * Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/diff/DiffIndex.java Co-authored-by: Amit Jain <amitj@apache.org> * Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/diff/JsonNodeBuilder.java Co-authored-by: Benjamin Habegger <benjamin@habegger.tech> * Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/diff/DiffIndex.java Co-authored-by: Amit Jain <amitj@apache.org> * OAK-12010 Simplified index management * OAK-12010 Simplified index management * OAK-12010 Simplified index management * OAK-12010 Simplified index management * OAK-12010 Simplified index management --------- Co-authored-by: Benjamin Habegger <benjamin@habegger.tech> Co-authored-by: Amit Jain <amitj@apache.org>

thomasmueller added 4 commits January 19, 2026 10:29

OAK-12010 Simplified index management (without optimizer)

a91de68

OAK-12010 Simplified index management

42d1e36

OAK-12010 Simplified index management

041af37

OAK-12010 Simplified index management

e2d6c72

bhabegger reviewed Jan 30, 2026

View reviewed changes

thomasmueller and others added 2 commits February 2, 2026 16:18

Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index…

a1c2fa9

…/diff/DiffIndex.java Co-authored-by: Benjamin Habegger <benjamin@habegger.tech>

Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index…

8ec6124

…/diff/DiffIndex.java Co-authored-by: Benjamin Habegger <benjamin@habegger.tech>

amit-jain approved these changes Feb 3, 2026

View reviewed changes

bhabegger reviewed Feb 4, 2026

View reviewed changes

Comment thread oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/diff/JsonNodeBuilder.java Outdated

thomasmueller and others added 8 commits February 6, 2026 15:25

Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index…

fbeab78

…/diff/DiffIndex.java Co-authored-by: Amit Jain <amitj@apache.org>

Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index…

2dfbe74

…/diff/DiffIndexMerger.java Co-authored-by: Amit Jain <amitj@apache.org>

Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index…

f9e6fec

…/diff/DiffIndex.java Co-authored-by: Amit Jain <amitj@apache.org>

Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index…

fcca185

…/diff/JsonNodeBuilder.java Co-authored-by: Benjamin Habegger <benjamin@habegger.tech>

Update oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index…

0f71ddd

…/diff/DiffIndex.java Co-authored-by: Amit Jain <amitj@apache.org>

OAK-12010 Simplified index management

aee4d17

OAK-12010 Simplified index management

7097f76

OAK-12010 Simplified index management

d954a1f

thomasmueller added 2 commits February 9, 2026 09:28

OAK-12010 Simplified index management

2f31a0b

OAK-12010 Simplified index management

fc19b5a

thomasmueller merged commit b00658b into trunk Feb 10, 2026

ChlineSaurus reviewed Feb 10, 2026

View reviewed changes

	String merged = new DiffIndexMerger(new String[0], true, true, false).processMerge(JsonObject.fromJson("{}"
	boolean deleteCreatesDummyIndex = true;
	String merged = new DiffIndexMerger(new String[0], deleteCreatesDummyIndex, true, false).processMerge(JsonObject.fromJson("{}"

Conversation

thomasmueller commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bhabegger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amit-jain left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Feb 6, 2026

Quality Gate passed

Uh oh!

ChlineSaurus left a comment

Choose a reason for hiding this comment

Uh oh!

ChlineSaurus Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thomasmueller commented Jan 19, 2026 •

edited

Loading

ChlineSaurus Feb 10, 2026 •

edited

Loading