[
https://issues.apache.org/jira/browse/OAK-7983?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16741579#comment-16741579
]
Vikas Saurabh commented on OAK-7983:
------------------------------------
[~tmueller], [~teofili] as I noted above that the issue/regression is due the
difference in behavior of {{LuceneProeprtyIndex#acquireIndexNode(String
indexPath)}} now always returning an object while earlier in case the index
wasn't available the return used to be {{null}}. The earlier case got guarded
at:
{noformat}
@Override
public List<IndexPlan> getPlans(Filter filter, List<OrderEntry> sortOrder,
NodeState rootState) {
Collection<String> indexPaths = new IndexLookup(rootState,
getIndexDefinitionPredicate())
.collectIndexNodePaths(filter);
List<IndexPlan> plans =
Lists.newArrayListWithCapacity(indexPaths.size());
for (String path : indexPaths) {
IndexNode indexNode = null;
try {
indexNode = acquireIndexNode(path);
if (indexNode != null) {
{noformat}
This implied that further calls like:
#
org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndex#getPlans
(the case where we coould delay opening index)
#
org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndex#getPlanDescription
#
org.apache.jackrabbit.oak.plugins.index.lucene.LucenePropertyIndex#getSizeEstimator
# com.google.common.collect.AbstractIterator#loadDocs
could correctly assume that a valid lucene index was sitting under the hood.
Of these the last 3 (cases 2, 3, and 4) are justified to need loaded index. The
first case is the only place where laziness in opening is ok (details at \[1]).
I think if use patch at \[2] and improve sanity test to also include case of
non-existing-:data-folder then we'd mostly be covered for happy cases. Genuine
corruption would continue to log NPE until the index gets marked as corrupted.
That could be ok I guess.
Btw, while we did see an empty :data folder in a an AEM instance - I couldn't
make a test to simulate that scenario (other that explicitly deleting
{{:data}}). Also, we noticed that reindexing that index (which had no nodes to
be stored in index) still had missing {{:data}} node. I couldn't get this
scenario going. Fwiw, explicit lucene index is written for no indexable nodes -
{{org.apache.jackrabbit.oak.plugins.index.lucene.writer.DefaultIndexWriter#close}}.
The differentiating factors in that AEM instance index were:
* the setup was working on composite node store
* the index had included paths = /home/users (part of read write store in CNS)
* no indexable nodes existed in the repository.
\[1]
{noformat}
org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndex#getPlans
needs index at
{{org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndexPlanner#getPlanBuilder}}:
# getNativeFunctionPlanBuilder -> defaultPlan() // bit early but still after
some sanity check on defs /* this can be avoided altogether as estimated cost
is overridden ... not worth it imo though */
# defaultPlan() // fairly late (called to get num docs)
{noformat}
While theoretically with {{entryCount}} in index def, we could avoid index open
completely, but that would complicate things too much without much RoI imo
\[2]
{noformat}
diff --git
a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java
b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java
index 4fa6dc652b..1b049a9d73 100644
---
a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java
+++
b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java
@@ -693,8 +693,8 @@ public class LucenePropertyIndex extends FulltextIndex {
}
@Override
- protected LazyLuceneIndexNode acquireIndexNode(String indexPath) {
- return new LazyLuceneIndexNode(tracker, indexPath);
+ protected LuceneIndexNode acquireIndexNode(String indexPath) {
+ return LazyLuceneIndexNode.getLuceneIndexNodeOrNull(tracker,
indexPath);
}
@Override
@@ -1571,12 +1571,27 @@ public class LucenePropertyIndex extends FulltextIndex {
private String indexPath;
private volatile LuceneIndexNode indexNode;
+ static LuceneIndexNode getLuceneIndexNodeOrNull(IndexTracker tracker,
String indexPath) {
+ if (NON_LAZY) {
+ return reallyAcquireIndexNode(tracker, indexPath);
+ }
+
+ // do some sanity checks
+ if(tracker.getBadIndexTracker().isIgnoredBadIndex(indexPath)) {
+ return null;
+ }
+
+ LazyLuceneIndexNode ret = new LazyLuceneIndexNode(tracker,
indexPath);
+ return ret;
+ }
+
+ static LuceneIndexNode reallyAcquireIndexNode(IndexTracker tracker,
String indexPath) {
+ return tracker.acquireIndexNode(indexPath);
+ }
+
LazyLuceneIndexNode(IndexTracker tracker, String indexPath) {
this.tracker = tracker;
this.indexPath = indexPath;
- if (NON_LAZY) {
- getIndexNode();
- }
}
@Override
@@ -1616,7 +1631,7 @@ public class LucenePropertyIndex extends FulltextIndex {
synchronized (this) {
n = indexNode;
if (n == null) {
- n = indexNode = tracker.acquireIndexNode(indexPath);
+ n = indexNode = reallyAcquireIndexNode(tracker,
indexPath);
}
}
}
@@ -1630,7 +1645,11 @@ public class LucenePropertyIndex extends FulltextIndex {
@Override
public LuceneIndexStatistics getIndexStatistics() {
- return getIndexNode().getIndexStatistics();
+ if (getIndexNode() != null) {
+ return getIndexNode().getIndexStatistics();
+ } else {
+ return null;
+ }
}
@Override
diff --git
a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java
b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java
index 3db264060b..3a1c3ba9d6 100644
---
a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java
+++
b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java
@@ -185,6 +185,14 @@ public class FulltextIndexPlanner {
if (definition.hasFunctionDefined()
&& filter.getPropertyRestriction(definition.getFunctionName())
!= null) {
+
+ // With OAK-7947 lucene indexes return a non-null index node to
delay reading index files
+ // While IndexNode could have a status check method but for now we
are using this work-around
+ // to check null on {@code getIndexStatistics()} as proxy indicator
+ if (indexNode.getIndexStatistics() == null) {
+ return null;
+ }
+
return
getNativeFunctionPlanBuilder(indexingRule.getBaseNodeType());
}
@@ -285,6 +293,13 @@ public class FulltextIndexPlanner {
int costPerEntryFactor = 1;
costPerEntryFactor += sortOrder.size();
+ // With OAK-7947 lucene indexes return a non-null index node to
delay reading index files
+ // While IndexNode could have a status check method but for now we
are using this work-around
+ // to check null on {@code getIndexStatistics()} as proxy indicator
+ if (indexNode.getIndexStatistics() == null) {
+ return null;
+ }
+
IndexPlan.Builder plan = defaultPlan();
if (!sortOrder.isEmpty()) {
plan.setSortOrder(sortOrder);
{noformat}
> LazyLuceneIndexNode#getIndexNode can cause NPE
> ----------------------------------------------
>
> Key: OAK-7983
> URL: https://issues.apache.org/jira/browse/OAK-7983
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: lucene
> Affects Versions: 1.9.13
> Reporter: Tommaso Teofili
> Priority: Major
> Attachments: OAK-7983.0.patch
>
>
> Changes for OAK-7947 have introduced a LazyLuceneIndexNode. Its methods call
> for IndexTracker#findIndexNode and #acquireIndexNode which can return `null`,
> however no proper checks are performed.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
