[
https://issues.apache.org/jira/browse/OAK-4623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15429539#comment-15429539
]
Vikas Saurabh edited comment on OAK-4623 at 8/21/16 12:10 AM:
--------------------------------------------------------------
[~mreutegg], I agree with logging cached (or fresh as the case may be) document
for the child - but I'm feeling a little uneasy for forcing a read with
maxCacheAge=0 as it'd update doc cache just for logging purposes. I mean, I
feel ok with invalidating a potential bad cache entry - but I won't want to
read it from remote just for logging. But, then I can also see the
counter-argument that this situation must never occur - so a penalty to read
remote for logging should be ok.
Also, I wonder if we should do reads in try{} to handle DocStore#find throwing
DocStoreException.
Here's a proposed diff btw:
{noformat}
diff --git
a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
index 1a909e7..05b0fb0 100644
---
a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
+++
b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
@@ -1059,7 +1059,25 @@ public final class DocumentNodeStore
String p = concat(parent.getPath(), input);
DocumentNodeState result = getNode(p, readRevision);
if (result == null) {
- throw new DocumentStoreException("DocumentNodeState is
null for revision " + readRevision + " of " + p + " (aborting
getChildNodes())");
+ //This is very unexpected situation - parent's child list
declares the child to exist, while
+ //its node state is null. Let's put some extra effort to
some logging
+ String cachedDocStr, uncachedDocStr;
+ try {
+ cachedDocStr = store.find(Collection.NODES,
Utils.getIdFromPath(p)).asString();
+ } catch (DocumentStoreException dse) {
+ cachedDocStr = dse.toString();
+
+ }
+ try {
+ uncachedDocStr = store.find(Collection.NODES,
Utils.getIdFromPath(p), 0).asString();
+ } catch (DocumentStoreException dse) {
+ uncachedDocStr = dse.toString();
+ }
+ String exceptionMsg = String.format(
+ "Aborting getChildNodes() - DocumentNodeState is
null for %s at %s " +
+ "{\"cachedDoc\":{%s},
\"uncachedDoc\":{%s})",
+ readRevision, p, cachedDocStr, uncachedDocStr);
+ throw new DocumentStoreException(exceptionMsg);
}
return result;
}
{noformat}
was (Author: catholicon):
[~mreutegg], I agree with logging cached (or fresh as the case may be) document
for the child - but I'm feeling a little uneasy for forcing a read with
maxCacheAge=0 as it'd update doc cache just for logging purposes. I mean, I
feel ok with invalidating a potential bad cache entry - but I won't want to
read it from remote just for logging. But, then I can also see the
counter-argument that this situation must never occur - so a penalty to read
remote for logging should be ok.
Also, I wonder if we should do reads in try{} to handle DocStore#find throwing
DocStoreException.
> Log more information when null DocumentNodeState is read for a child while
> fetching children
> --------------------------------------------------------------------------------------------
>
> Key: OAK-4623
> URL: https://issues.apache.org/jira/browse/OAK-4623
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: documentmk
> Reporter: Vikas Saurabh
> Assignee: Vikas Saurabh
> Priority: Minor
> Fix For: 1.6
>
>
> {{DocumentNodeStore#getChildNodes}} validates node states of children in the
> returned iterator and throws exception if a child is read as {{null}}.
> This is clearly an unexpected case (although, it has been observed in running
> systems). The more probable cause for such a scenario would be to have
> inconsistent value in {{nodeChildrenCache}}. It'd be useful to log more
> information about cache state to help investigating such cases.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
