You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2021/11/24 14:15:27 UTC
[GitHub] [jackrabbit-oak] fabriziofortino commented on a change in pull request #420: OAK-9624 print the name of the calling class invoking a query in some cases
fabriziofortino commented on a change in pull request #420:
URL: https://github.com/apache/jackrabbit-oak/pull/420#discussion_r755913415
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUtils.java
##########
@@ -30,16 +30,19 @@
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.UNIQUE_PROPERTY_NAME;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Map;
import java.util.Set;
+import java.util.stream.Stream;
import javax.jcr.RepositoryException;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.api.QueryEngine;
Review comment:
not used
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUtils.java
##########
@@ -30,16 +30,19 @@
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.UNIQUE_PROPERTY_NAME;
+import java.util.Arrays;
Review comment:
not used
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/Cursors.java
##########
@@ -339,7 +340,9 @@ private void fetchNext() {
readCount++;
if (readCount % 1000 == 0) {
FilterIterators.checkReadLimit(readCount, settings);
- LOG.warn("Traversed " + readCount + " nodes with filter " + filter + "; consider creating an index or changing the query");
+ String caller = IndexUtils.getCaller(this.settings.getIgnoredClassNamesInCallTrace());
+ LOG.warn("Traversed {} nodes with filter {} called by {}; consider creating an index or changing the query" ,
+ new Object[] {readCount, filter, caller});
Review comment:
the array creation could be avoided since the method receives a `varargs`
```suggestion
LOG.warn("Traversed {} nodes with filter {} called by {}; consider creating an index or changing the query" ,
readCount, filter, caller);
```
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUtils.java
##########
@@ -241,4 +244,37 @@ public static String getAsyncLaneName(NodeState idxState, String indexPath, Prop
}
return null;
}
+
+ /**
+ * Retrieves the calling class and method from the call stack; this is determined by unwinding
+ * the stack until it finds a combination of full qualified classname + method (separated by ".") which
+ * do not start with any of the values provided by the ignoredJavaPackages parameters. If the provided
+ * parameters cover all stack frames, the whole query is considered to be internal, where the
+ * actual caller doesn't matter (or cannot be determined clearly). In this case a short message
+ * indicating this is returned.
+ *
+ * If the ingoredJavaPackages parameter is null or empty, the caller is not looked up, but
Review comment:
typo
```suggestion
* If the ignoredJavaPackages parameter is null or empty, the caller is not looked up, but
```
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/Cursors.java
##########
@@ -16,6 +16,7 @@
*/
package org.apache.jackrabbit.oak.plugins.index;
+import java.util.Arrays;
Review comment:
minor: this import is not needed
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUtils.java
##########
@@ -241,4 +244,37 @@ public static String getAsyncLaneName(NodeState idxState, String indexPath, Prop
}
return null;
}
+
+ /**
+ * Retrieves the calling class and method from the call stack; this is determined by unwinding
+ * the stack until it finds a combination of full qualified classname + method (separated by ".") which
+ * do not start with any of the values provided by the ignoredJavaPackages parameters. If the provided
+ * parameters cover all stack frames, the whole query is considered to be internal, where the
+ * actual caller doesn't matter (or cannot be determined clearly). In this case a short message
+ * indicating this is returned.
+ *
+ * If the ingoredJavaPackages parameter is null or empty, the caller is not looked up, but
+ * instead it is assumed, that the feature is not configured; in that case a short messages
+ * is returned indicating that the feature is not configured.
+ *
+ * @param ignoredJavaPackages the java packages or class names
+ * @return the calling class or another non-null value
+ */
+ @NotNull
+ public static String getCaller(@Nullable String[] ignoredJavaPackages) {
+ if (ignoredJavaPackages == null || ignoredJavaPackages.length == 0) {
+ return "(<function not configured>)";
+ }
+
+ // With java9 we would use https://docs.oracle.com/javase/9/docs/api/java/lang/StackWalker.html
+ final StackTraceElement[] callStack = Thread.currentThread().getStackTrace();
+ for (StackTraceElement stackFrame : callStack) {
+ final String classAndMethod = stackFrame.getClassName() + "." + stackFrame.getMethodName();
+ if (Stream.of(ignoredJavaPackages).noneMatch(pkg -> classAndMethod.startsWith(pkg))) {
Review comment:
you could use method reference here but I think it's a matter of taste
```suggestion
if (Stream.of(ignoredJavaPackages).noneMatch(classAndMethod::startsWith)) {
```
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettingsService.java
##########
@@ -77,6 +77,14 @@
"the queryPaths of the index is taken into account."
)
String getStrictPathRestrictionsForIndexes() default DISABLED_STRICT_PATH_RESTRICTION;
+
+ @AttributeDefinition(
+ name="Fully qualified class names to ignore when finding caller",
+ description="If non-empty the query engine logs the query statement plus the java package "
+ + "which executed this query. This java package is the first package in the call trace"
+ + "which does not not start with the any of the provided fully qualfied class names (packagename + classname)"
Review comment:
typos
```suggestion
+ "which does not start with any of the provided fully qualified class names (packagename + classname)"
```
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettingsService.java
##########
@@ -77,6 +77,14 @@
"the queryPaths of the index is taken into account."
)
String getStrictPathRestrictionsForIndexes() default DISABLED_STRICT_PATH_RESTRICTION;
+
+ @AttributeDefinition(
+ name="Fully qualified class names to ignore when finding caller",
+ description="If non-empty the query engine logs the query statement plus the java package "
+ + "which executed this query. This java package is the first package in the call trace"
Review comment:
missing space at the end
```suggestion
+ "which executed this query. This java package is the first package in the call trace "
```
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java
##########
@@ -24,12 +24,14 @@
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_CONTENT_NODE_NAME;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.PROPERTY_NAMES;
+import java.util.Arrays;
Review comment:
not used
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUtils.java
##########
@@ -241,4 +244,37 @@ public static String getAsyncLaneName(NodeState idxState, String indexPath, Prop
}
return null;
}
+
+ /**
+ * Retrieves the calling class and method from the call stack; this is determined by unwinding
+ * the stack until it finds a combination of full qualified classname + method (separated by ".") which
+ * do not start with any of the values provided by the ignoredJavaPackages parameters. If the provided
+ * parameters cover all stack frames, the whole query is considered to be internal, where the
+ * actual caller doesn't matter (or cannot be determined clearly). In this case a short message
+ * indicating this is returned.
+ *
+ * If the ingoredJavaPackages parameter is null or empty, the caller is not looked up, but
+ * instead it is assumed, that the feature is not configured; in that case a short messages
+ * is returned indicating that the feature is not configured.
+ *
+ * @param ignoredJavaPackages the java packages or class names
+ * @return the calling class or another non-null value
+ */
+ @NotNull
+ public static String getCaller(@Nullable String[] ignoredJavaPackages) {
+ if (ignoredJavaPackages == null || ignoredJavaPackages.length == 0) {
+ return "(<function not configured>)";
+ }
+
+ // With java9 we would use https://docs.oracle.com/javase/9/docs/api/java/lang/StackWalker.html
+ final StackTraceElement[] callStack = Thread.currentThread().getStackTrace();
+ for (StackTraceElement stackFrame : callStack) {
+ final String classAndMethod = stackFrame.getClassName() + "." + stackFrame.getMethodName();
+ if (Stream.of(ignoredJavaPackages).noneMatch(pkg -> classAndMethod.startsWith(pkg))) {
+ return classAndMethod;
+ }
+ }
+ // if every element is ignored, we assume it's an internal request
+ return "(internal)";
Review comment:
If you want to be more `functional` this should produce the same results. Notice that the stream is lazy, the `findFirst` basically returns the first element filtered in the previous call without filtering the entire stream. WARN: I have not tested it :-)
```suggestion
return Arrays.stream(Thread.currentThread().getStackTrace())
.map(sf -> sf.getClassName() + "." + sf.getMethodName())
.filter(classAndMethod -> Stream.of(ignoredJavaPackages).noneMatch(classAndMethod::startsWith))
.findFirst()
.orElse("(internal)");
```
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettingsService.java
##########
@@ -77,6 +77,14 @@
"the queryPaths of the index is taken into account."
)
String getStrictPathRestrictionsForIndexes() default DISABLED_STRICT_PATH_RESTRICTION;
+
+ @AttributeDefinition(
+ name="Fully qualified class names to ignore when finding caller",
+ description="If non-empty the query engine logs the query statement plus the java package "
+ + "which executed this query. This java package is the first package in the call trace"
+ + "which does not not start with the any of the provided fully qualfied class names (packagename + classname)"
+ )
+ String[] ignoredClassNamesinCallTrace() default {};
Review comment:
this should be camel cased
```suggestion
String[] ignoredClassNamesInCallTrace() default {};
```
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org