fabriziofortino commented on a change in pull request #370:
URL: https://github.com/apache/jackrabbit-oak/pull/370#discussion_r707177275
##########
File path:
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexInfoProvider.java
##########
@@ -140,6 +139,24 @@ private static void computeLastUpdatedTime(NodeState
idxState, LuceneIndexInfo i
}
}
+ private static void checkIfHiddenNodesExists(NodeState idxState,
LuceneIndexInfo info) {
+ // Check for hidden oak libs mount node that has indexed content for
read only repo in composite store
+ info.hasHiddenOakLibsMount = false;
+ for(String c : idxState.getChildNodeNames()) {
+ if (c.startsWith(IndexDefinition.HIDDEN_OAK_MOUNT_PREFIX)) {
+ info.hasHiddenOakLibsMount = true;
+ }
+ }
+
+ // Now check for hidden property index node :property-index - present
in case of hybrid indexes
+ info.hasPropertyIndexNode = false;
+ for(String c : idxState.getChildNodeNames()) {
+ if (c.equals(IndexDefinition.PROPERTY_INDEX)) {
+ info.hasPropertyIndexNode = true;
Review comment:
`break` ?
##########
File path:
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexInfoProvider.java
##########
@@ -140,6 +139,24 @@ private static void computeLastUpdatedTime(NodeState
idxState, LuceneIndexInfo i
}
}
+ private static void checkIfHiddenNodesExists(NodeState idxState,
LuceneIndexInfo info) {
+ // Check for hidden oak libs mount node that has indexed content for
read only repo in composite store
+ info.hasHiddenOakLibsMount = false;
+ for(String c : idxState.getChildNodeNames()) {
+ if (c.startsWith(IndexDefinition.HIDDEN_OAK_MOUNT_PREFIX)) {
+ info.hasHiddenOakLibsMount = true;
Review comment:
`break`?
##########
File path:
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexMBeanImpl.java
##########
@@ -122,13 +133,13 @@ public TabularData getIndexStats() throws IOException {
TabularType tt = new
TabularType(LuceneIndexMBeanImpl.class.getName(),
"Lucene Index Stats", IndexStats.TYPE, new
String[]{"path"});
tds = new TabularDataSupport(tt);
- Set<String> indexes = indexTracker.getIndexNodePaths();
- for (String path : indexes) {
+ // Use indexPathService to get list of all the lucene indexes.
Review comment:
If I get it right, this is needed to show all the indexes while the
`IndexTracker` returns the open ones only. Can we explain this in the comment?
##########
File path:
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexInfoProvider.java
##########
@@ -140,6 +139,24 @@ private static void computeLastUpdatedTime(NodeState
idxState, LuceneIndexInfo i
}
}
+ private static void checkIfHiddenNodesExists(NodeState idxState,
LuceneIndexInfo info) {
+ // Check for hidden oak libs mount node that has indexed content for
read only repo in composite store
+ info.hasHiddenOakLibsMount = false;
+ for(String c : idxState.getChildNodeNames()) {
+ if (c.startsWith(IndexDefinition.HIDDEN_OAK_MOUNT_PREFIX)) {
+ info.hasHiddenOakLibsMount = true;
+ }
+ }
+
+ // Now check for hidden property index node :property-index - present
in case of hybrid indexes
+ info.hasPropertyIndexNode = false;
+ for(String c : idxState.getChildNodeNames()) {
+ if (c.equals(IndexDefinition.PROPERTY_INDEX)) {
+ info.hasPropertyIndexNode = true;
+ }
+ }
+ }
Review comment:
Apart from terminating the conditions with a break, I think this logic
can be simplified with a single for-loop.
##########
File path:
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexMBeanImpl.java
##########
@@ -269,6 +280,10 @@ public boolean isFailing() {
return getFieldTermPrefixInfo(indexPath, field, Integer.MAX_VALUE,
term);
}
+ private void bindIndexInfoProviders(IndexInfoServiceImpl indexInfoService)
{
Review comment:
this method is called only in the constructor. To improve the
readability, I would move this logic in the constructor so the reader does not
have to jump somewhere else to see what it does.
##########
File path:
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexMBeanImpl.java
##########
@@ -675,7 +690,10 @@ private static Query newDepthQuery(int depth) {
"numDeletedDocs",
"nrtIndexSize",
"nrtIndexSizeStr",
- "nrtNumDocs"
+ "nrtNumDocs",
+ "lastUpdatedTimeStamp",
Review comment:
Timestamp can be written with one or two words. Usually, in software,
the single word version is used.
##########
File path:
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexInfoProvider.java
##########
@@ -140,6 +139,24 @@ private static void computeLastUpdatedTime(NodeState
idxState, LuceneIndexInfo i
}
}
+ private static void checkIfHiddenNodesExists(NodeState idxState,
LuceneIndexInfo info) {
+ // Check for hidden oak libs mount node that has indexed content for
read only repo in composite store
+ info.hasHiddenOakLibsMount = false;
+ for(String c : idxState.getChildNodeNames()) {
+ if (c.startsWith(IndexDefinition.HIDDEN_OAK_MOUNT_PREFIX)) {
Review comment:
I am not sure about this condition here. Instead of `startsWith`
shouldn't we check for a node with name `:oak:mount-libs-index-data`? cc
@thomasmueller
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]