Skip to content

Commit 0550fd6

Browse files
nfsantosnfsantos
authored
OAK-11538 - Cleanups and small performance improvements to CompositeEditor and CompositeIndexEditorProvider
1 parent 6c2f146 commit 0550fd6

File tree

16 files changed

+119
-129
lines changed

16 files changed

+119
-129
lines changed

‎oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/CompositeIndexEditorProvider.java‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,32 +16,44 @@
1616
*/
1717
package org.apache.jackrabbit.oak.plugins.index;
1818

19-
import java.io.IOException;
20-
import java.util.ArrayList;
21-
import java.util.Arrays;
22-
import java.util.Collection;
23-
import java.util.List;
24-
2519
import org.apache.jackrabbit.oak.api.CommitFailedException;
2620
import org.apache.jackrabbit.oak.spi.commit.CompositeEditor;
2721
import org.apache.jackrabbit.oak.spi.commit.Editor;
2822
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
2923
import org.apache.jackrabbit.oak.spi.state.NodeState;
3024
import org.jetbrains.annotations.NotNull;
3125

26+
import java.io.IOException;
27+
import java.util.ArrayList;
28+
import java.util.Arrays;
29+
import java.util.List;
30+
3231
/**
3332
* Aggregation of a list of editor providers into a single provider.
3433
*/
3534
public class CompositeIndexEditorProvider implements IndexEditorProvider {
3635

3736
@NotNull
38-
public static IndexEditorProvider compose(@NotNull Collection<IndexEditorProvider> providers) {
39-
if (providers.isEmpty()) {
40-
return (type, builder, root, callback) -> null;
41-
} else if (providers.size() == 1) {
42-
return providers.iterator().next();
43-
} else {
44-
return new CompositeIndexEditorProvider(List.copyOf(providers));
37+
public static IndexEditorProvider compose(IndexEditorProvider... providers) {
38+
switch (providers.length) {
39+
case 0:
40+
return (type, builder, root, callback) -> null;
41+
case 1:
42+
return providers[0];
43+
default:
44+
return new CompositeIndexEditorProvider(providers);
45+
}
46+
}
47+
48+
@NotNull
49+
public static IndexEditorProvider compose(@NotNull List<IndexEditorProvider> providers) {
50+
switch (providers.size()) {
51+
case 0:
52+
return (type, builder, root, callback) -> null;
53+
case 1:
54+
return providers.iterator().next();
55+
default:
56+
return new CompositeIndexEditorProvider(providers);
4557
}
4658
}
4759

‎oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@
3939
import java.util.Set;
4040
import java.util.concurrent.atomic.AtomicLong;
4141

42-
import org.apache.jackrabbit.guava.common.collect.Iterables;
43-
4442
import org.apache.jackrabbit.JcrConstants;
4543
import org.apache.jackrabbit.oak.api.CommitFailedException;
4644
import org.apache.jackrabbit.oak.api.PropertyState;
@@ -184,7 +182,7 @@ public void enter(NodeState before, NodeState after)
184182

185183
// no-op when reindex is empty
186184
CommitFailedException exception = EditorDiff.process(
187-
VisibleEditor.wrap(wrapProgress(CompositeEditor.compose(reindex.values()))),
185+
VisibleEditor.wrap(wrapProgress(CompositeEditor.compose(List.copyOf(reindex.values())))),
188186
MISSING_NODE,
189187
after);
190188
rootState.progressReporter.reindexingTraversalEnd();
@@ -197,31 +195,31 @@ public void enter(NodeState before, NodeState after)
197195
}
198196
}
199197

200-
public boolean isReindexingPerformed(){
198+
public boolean isReindexingPerformed() {
201199
return !getReindexStats().isEmpty();
202200
}
203201

204-
public List<String> getReindexStats(){
202+
public List<String> getReindexStats() {
205203
return rootState.progressReporter.getReindexStats();
206204
}
207205

208-
public Set<String> getUpdatedIndexPaths(){
206+
public Set<String> getUpdatedIndexPaths() {
209207
return rootState.progressReporter.getUpdatedIndexPaths();
210208
}
211209

212-
public void setTraversalRateEstimator(TraversalRateEstimator estimator){
210+
public void setTraversalRateEstimator(TraversalRateEstimator estimator) {
213211
rootState.progressReporter.setTraversalRateEstimator(estimator);
214212
}
215213

216-
public void setNodeCountEstimator(NodeCountEstimator nodeCountEstimator){
214+
public void setNodeCountEstimator(NodeCountEstimator nodeCountEstimator) {
217215
rootState.progressReporter.setNodeCountEstimator(nodeCountEstimator);
218216
}
219217

220-
public String getIndexingStats(){
218+
public String getIndexingStats() {
221219
return rootState.getIndexingStats();
222220
}
223221

224-
public void setIgnoreReindexFlags(boolean ignoreReindexFlag){
222+
public void setIgnoreReindexFlags(boolean ignoreReindexFlag) {
225223
rootState.setIgnoreReindexFlags(ignoreReindexFlag);
226224
}
227225

@@ -271,7 +269,7 @@ private boolean shouldReindex(NodeBuilder definition, NodeState before, String n
271269
return result;
272270
}
273271

274-
private static boolean hasAnyHiddenNodes(NodeBuilder builder){
272+
private static boolean hasAnyHiddenNodes(NodeBuilder builder) {
275273
for (String name : builder.getChildNodeNames()) {
276274
if (NodeStateUtils.isHidden(name)) {
277275
NodeBuilder childNode = builder.getChildNode(name);
@@ -284,8 +282,7 @@ private static boolean hasAnyHiddenNodes(NodeBuilder builder){
284282
return false;
285283
}
286284

287-
private void collectIndexEditors(NodeBuilder definitions,
288-
NodeState before) throws CommitFailedException {
285+
private void collectIndexEditors(NodeBuilder definitions, NodeState before) throws CommitFailedException {
289286
for (String name : definitions.getChildNodeNames()) {
290287
NodeBuilder definition = definitions.getChildNode(name);
291288
if (isIncluded(rootState.async, definition)) {
@@ -312,7 +309,7 @@ private void collectIndexEditors(NodeBuilder definitions,
312309

313310
boolean shouldReindex = shouldReindex(definition, before, name);
314311
String indexPath = getIndexPath(getPath(), name);
315-
if (definition.hasProperty(IndexConstants.CORRUPT_PROPERTY_NAME) && !shouldReindex){
312+
if (definition.hasProperty(IndexConstants.CORRUPT_PROPERTY_NAME) && !shouldReindex) {
316313
String corruptSince = definition.getProperty(IndexConstants.CORRUPT_PROPERTY_NAME).getValue(Type.DATE);
317314
rootState.corruptIndexHandler.skippingCorruptIndex(rootState.async, indexPath, ISO8601.parse(corruptSince));
318315
continue;
@@ -341,7 +338,7 @@ private void collectIndexEditors(NodeBuilder definitions,
341338
long now = System.nanoTime();
342339
long last = lastMissingProviderMessageTime.get();
343340
if (now > last + silenceMessagesNanos
344-
&& lastMissingProviderMessageTime.compareAndSet(last, now)) {
341+
&& lastMissingProviderMessageTime.compareAndSet(last, now)) {
345342
log.warn("Missing provider for nrt/sync index: {}. " +
346343
"Please note, it means that index data should be trusted only after this index " +
347344
"is processed in an async indexing cycle. " +
@@ -429,7 +426,7 @@ static boolean isIncluded(String asyncRef, NodeBuilder definition) {
429426
* <p>Note that this differs from #isIncluded which also considers the value of <code>async</code>
430427
* property to determine if the index should be selected for current IndexUpdate run.
431428
*/
432-
private boolean isMatchingIndexMode(NodeBuilder definition){
429+
private boolean isMatchingIndexMode(NodeBuilder definition) {
433430
boolean async = definition.hasProperty(ASYNC_PROPERTY_NAME);
434431
//Either
435432
// 1. async index and async index update
@@ -439,7 +436,7 @@ private boolean isMatchingIndexMode(NodeBuilder definition){
439436

440437
private void incrementReIndexCount(NodeBuilder definition) {
441438
long count = 0;
442-
if(definition.hasProperty(REINDEX_COUNT)){
439+
if (definition.hasProperty(REINDEX_COUNT)) {
443440
count = definition.getProperty(REINDEX_COUNT).getValue(Type.LONG);
444441
}
445442
definition.setProperty(REINDEX_COUNT, count + 1);
@@ -463,7 +460,7 @@ public void leave(NodeState before, NodeState after)
463460
editor.leave(before, after);
464461
}
465462

466-
if (parent == null){
463+
if (parent == null) {
467464
rootState.progressReporter.logReport();
468465
}
469466
}
@@ -562,7 +559,7 @@ private static String getIndexPath(String path, String indexName) {
562559
return path + "/" + INDEX_DEFINITIONS_NAME + "/" + indexName;
563560
}
564561

565-
private Editor wrapProgress(Editor editor){
562+
private Editor wrapProgress(Editor editor) {
566563
return rootState.progressReporter.wrapProgress(editor);
567564
}
568565

@@ -651,7 +648,7 @@ public IndexUpdateCallback newCallback(String indexPath, boolean reindex, long e
651648
return new ReportingCallback(indexPath, reindex);
652649
}
653650

654-
public boolean isAsync(){
651+
public boolean isAsync() {
655652
return async != null;
656653
}
657654

@@ -660,7 +657,7 @@ public void nodeRead(PathSource pathSource) throws CommitFailedException {
660657
progressReporter.traversedNode(pathSource);
661658
}
662659

663-
public void propertyChanged(String name){
660+
public void propertyChanged(String name) {
664661
changedPropertyCount++;
665662
}
666663

@@ -702,7 +699,7 @@ public ReportingCallback(String indexPath, boolean reindex) {
702699

703700
@Override
704701
public void indexUpdate() throws CommitFailedException {
705-
progressReporter.indexUpdate(indexPath);
702+
progressReporter.indexUpdate(indexPath);
706703
}
707704

708705
//~------------------------------< ContextAwareCallback >

‎oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/NodeStoreUtils.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,13 @@
2929
import org.apache.jackrabbit.oak.plugins.index.IndexUpdateProvider;
3030
import org.apache.jackrabbit.oak.spi.commit.CommitContext;
3131
import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
32-
import org.apache.jackrabbit.oak.spi.commit.CompositeEditorProvider;
3332
import org.apache.jackrabbit.oak.spi.commit.CompositeHook;
3433
import org.apache.jackrabbit.oak.spi.commit.EditorHook;
3534
import org.apache.jackrabbit.oak.spi.commit.ResetCommitAttributeHook;
3635
import org.apache.jackrabbit.oak.spi.commit.SimpleCommitContext;
3736
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
3837
import org.apache.jackrabbit.oak.spi.state.NodeStore;
3938

40-
import static java.util.Collections.singletonList;
4139
import static java.util.Objects.requireNonNull;
4240

4341
final class NodeStoreUtils {
@@ -46,7 +44,7 @@ static void mergeWithConcurrentCheck(NodeStore nodeStore, NodeBuilder builder) t
4644
CompositeHook hooks = new CompositeHook(
4745
ResetCommitAttributeHook.INSTANCE,
4846
new ConflictHook(new AnnotatingConflictHandler()),
49-
new EditorHook(CompositeEditorProvider.compose(singletonList(new ConflictValidatorProvider())))
47+
new EditorHook(new ConflictValidatorProvider())
5048
);
5149
nodeStore.merge(builder, hooks, createCommitInfo());
5250
}
@@ -57,7 +55,7 @@ static void mergeWithConcurrentCheck(NodeStore nodeStore, NodeBuilder builder,
5755
ResetCommitAttributeHook.INSTANCE,
5856
new EditorHook(new IndexUpdateProvider(indexEditorProvider, null, true)),
5957
new ConflictHook(new AnnotatingConflictHandler()),
60-
new EditorHook(CompositeEditorProvider.compose(singletonList(new ConflictValidatorProvider())))
58+
new EditorHook(new ConflictValidatorProvider())
6159
);
6260
nodeStore.merge(builder, hooks, createCommitInfo());
6361
}

‎oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,8 +1032,7 @@ public void testReindexMissingProvider() throws Exception {
10321032
.getString(missingAsync));
10331033

10341034
// second run, simulate an index going away
1035-
provider = CompositeIndexEditorProvider
1036-
.compose(new ArrayList<IndexEditorProvider>());
1035+
provider = CompositeIndexEditorProvider.compose();
10371036
async = new AsyncIndexUpdate(missingAsync, store, provider);
10381037
async.run();
10391038
assertTrue(async.isFailing());
@@ -1083,7 +1082,7 @@ public void testReindexMissingProvider_NonRoot() throws Exception {
10831082
store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
10841083

10851084
// second run, simulate an index going away
1086-
provider = CompositeIndexEditorProvider.compose(new ArrayList<IndexEditorProvider>());
1085+
provider = CompositeIndexEditorProvider.compose();
10871086
async = new AsyncIndexUpdate(missingAsyncName, store, provider);
10881087
async.run();
10891088
assertTrue(async.isFailing());

‎oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/AsyncIndexStatsUpdateCallbackTest.java‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.apache.jackrabbit.oak.api.Root;
2727
import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
2828
import org.apache.jackrabbit.oak.plugins.index.AsyncIndexUpdate;
29+
import org.apache.jackrabbit.oak.plugins.index.CompositeIndexEditorProvider;
2930
import org.apache.jackrabbit.oak.plugins.index.counter.NodeCounterEditorProvider;
3031
import org.apache.jackrabbit.oak.plugins.index.lucene.directory.ActiveDeletedBlobCollectorFactory;
3132
import org.apache.jackrabbit.oak.plugins.index.lucene.util.LuceneIndexDefinitionBuilder;
@@ -42,26 +43,22 @@
4243
import java.io.IOException;
4344
import java.util.List;
4445

45-
import static org.apache.jackrabbit.oak.plugins.index.CompositeIndexEditorProvider.compose;
4646
import static org.hamcrest.CoreMatchers.containsString;
47-
import static org.junit.Assert.assertFalse;
48-
import static org.junit.Assert.assertNotNull;
49-
import static org.junit.Assert.assertNull;
47+
import static org.junit.Assert.assertEquals;
5048
import static org.junit.Assert.assertThat;
51-
import static org.junit.Assert.assertTrue;
5249

5350
/**
5451
* Tests AsyncIndexStatsUpdateCallback works as scheduled callback
5552
*/
5653
public class AsyncIndexStatsUpdateCallbackTest {
57-
private int SCHEDULED_CALLBACK_TIME_IN_MILLIS = 1000; //1 second
54+
private final int SCHEDULED_CALLBACK_TIME_IN_MILLIS = 1000; //1 second
5855
protected Root root;
5956
private AsyncIndexUpdate asyncIndexUpdate;
6057
private LuceneIndexEditorProvider luceneIndexEditorProvider;
6158
private LogCustomizer customLogger;
62-
private AsyncIndexesSizeStatsUpdateImpl asyncIndexesSizeStatsUpdate =
59+
private final AsyncIndexesSizeStatsUpdateImpl asyncIndexesSizeStatsUpdate =
6360
new AsyncIndexesSizeStatsUpdateImpl(SCHEDULED_CALLBACK_TIME_IN_MILLIS);
64-
private LuceneIndexMBeanImpl mBean = Mockito.mock(LuceneIndexMBeanImpl.class);
61+
private final LuceneIndexMBeanImpl mBean = Mockito.mock(LuceneIndexMBeanImpl.class);
6562

6663
@Before
6764
public void before() throws Exception {
@@ -87,10 +84,10 @@ protected ContentRepository createRepository() throws IOException {
8784
null, Mounts.defaultMountInfoProvider(),
8885
ActiveDeletedBlobCollectorFactory.NOOP, mBean, StatisticsProvider.NOOP);
8986

90-
asyncIndexUpdate = new AsyncIndexUpdate("async", nodeStore, compose(List.of(
87+
asyncIndexUpdate = new AsyncIndexUpdate("async", nodeStore, CompositeIndexEditorProvider.compose(
9188
luceneIndexEditorProvider,
9289
new NodeCounterEditorProvider()
93-
)), StatisticsProvider.NOOP, false);
90+
), StatisticsProvider.NOOP, false);
9491
return new Oak(nodeStore)
9592
.with(new InitialContent())
9693
.with(new OpenSecurityProvider())
@@ -111,16 +108,16 @@ public void testCallbackWorksAsScheduled() throws Exception {
111108
customLogger.starting();
112109
asyncIndexUpdate.run();
113110
List<String> logs = customLogger.getLogs();
114-
assertTrue(logs.size() == 1);
111+
assertEquals(1, logs.size());
115112
root.getTree("/content").addChild("c2").setProperty("foo", "bar");
116113
root.commit();
117114
asyncIndexUpdate.run();
118-
assertTrue(logs.size() == 1);
115+
assertEquals(1, logs.size());
119116
root.getTree("/content").addChild("c3").setProperty("foo", "bar");
120117
root.commit();
121118
Thread.sleep(2000);
122119
asyncIndexUpdate.run();
123-
assertTrue(logs.size() == 2);
120+
assertEquals(2, logs.size());
124121
validateLogs(logs);
125122
}
126123

@@ -138,16 +135,16 @@ public void testNOOPDonotPerformCallbackStatsUpdate() throws Exception {
138135
customLogger.starting();
139136
asyncIndexUpdate.run();
140137
List<String> logs = customLogger.getLogs();
141-
assertTrue(logs.size() == 0);
138+
assertEquals(0, logs.size());
142139
root.getTree("/content").addChild("c2").setProperty("foo", "bar");
143140
root.commit();
144141
asyncIndexUpdate.run();
145-
assertTrue(logs.size() == 0);
142+
assertEquals(0, logs.size());
146143
root.getTree("/content").addChild("c3").setProperty("foo", "bar");
147144
root.commit();
148145
Thread.sleep(2000);
149146
asyncIndexUpdate.run();
150-
assertTrue(logs.size() == 0);
147+
assertEquals(0, logs.size());
151148
//validateLogs(logs);
152149
}
153150

‎oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/AsyncIndexUpdateCorruptMarkingTest.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.apache.jackrabbit.oak.api.ContentSession;
2525
import org.apache.jackrabbit.oak.api.Root;
2626
import org.apache.jackrabbit.oak.plugins.index.AsyncIndexUpdate;
27+
import org.apache.jackrabbit.oak.plugins.index.CompositeIndexEditorProvider;
2728
import org.apache.jackrabbit.oak.plugins.index.TrackingCorruptIndexHandler;
2829
import org.apache.jackrabbit.oak.plugins.index.counter.NodeCounterEditorProvider;
2930
import org.apache.jackrabbit.oak.plugins.index.lucene.util.LuceneIndexDefinitionBuilder;
@@ -39,10 +40,8 @@
3940
import org.junit.Test;
4041

4142
import java.io.IOException;
42-
import java.util.List;
4343
import java.util.concurrent.TimeUnit;
4444

45-
import static org.apache.jackrabbit.oak.plugins.index.CompositeIndexEditorProvider.compose;
4645
import static org.junit.Assert.assertFalse;
4746
import static org.junit.Assert.assertNotNull;
4847
import static org.junit.Assert.assertNull;
@@ -76,10 +75,10 @@ protected ContentRepository createRepository() {
7675
LuceneIndexProvider provider = new LuceneIndexProvider();
7776
luceneIndexEditorProvider.setBlobStore(blobStore);
7877

79-
asyncIndexUpdate = new AsyncIndexUpdate("async", nodeStore, compose(List.of(
78+
asyncIndexUpdate = new AsyncIndexUpdate("async", nodeStore, CompositeIndexEditorProvider.compose(
8079
luceneIndexEditorProvider,
8180
new NodeCounterEditorProvider()
82-
)));
81+
));
8382
TrackingCorruptIndexHandler trackingCorruptIndexHandler = new TrackingCorruptIndexHandler();
8483
trackingCorruptIndexHandler.setCorruptInterval(INDEX_CORRUPT_INTERVAL_IN_MILLIS, TimeUnit.MILLISECONDS);
8584
asyncIndexUpdate.setCorruptIndexHandler(trackingCorruptIndexHandler);

0 commit comments

Comments
 (0)