You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by xristy <gi...@git.apache.org> on 2017/11/29 15:20:38 UTC
[GitHub] jena pull request #319: Jena 1441 tracing
GitHub user xristy opened a pull request:
https://github.com/apache/jena/pull/319
Jena 1441 tracing
Opening new to not include JENA-1438 related commit.
This adds tracing in TextQueryPF and TextIndexLucene to track calls to exec and subsequent processing as well as the add, update and delete of Lucene documents.
Also adds configuration item text:cacheQueries on text:TextIndexLucene which allows to bypass use of the queryCache in TextQueryPF.query(...) during development. This configuration item is not for general use and need not be documented further. text:cacheQueries defaults to true so there is no net change in the behavior of jena-text.
There is also tidying of code to use doc.get() rather than doc.getValues() followed by vals[0] in TextLuceneIndex.query$.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/BuddhistDigitalResourceCenter/jena JENA-1441-tracing
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/319.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #319
----
commit 7b016291a17cdcabb1cf7222094e1f00344ddf15
Author: Chris Tomlinson <ct...@moonvine.org>
Date: 2017-11-19T17:48:42Z
added config for using text cache; added debug log for Lucene docs
commit e2d6b89e369530a1c53d4579d500daa135439ab8
Author: Chris Tomlinson <ct...@moonvine.org>
Date: 2017-11-24T22:09:41Z
Merge remote-tracking branch 'upstream/master' into highlighting
commit e14457115e638b33dcd8923f22c2ff3655309ab0
Author: Chris Tomlinson <ct...@moonvine.org>
Date: 2017-11-25T00:22:58Z
use get vs getValues in query$; log trace for Lucene dev
commit 5e95a09eccaf7f8fa5ec659bebde83cdf8398163
Author: Chris Tomlinson <ct...@moonvine.org>
Date: 2017-11-26T16:19:11Z
Merge remote-tracking branch 'upstream/master' into highlighting
commit d98b096620ef09d115f745c9ecd1359d0a62c83e
Author: Chris Tomlinson <ct...@moonvine.org>
Date: 2017-11-28T17:52:50Z
Merge branch 'master' of https://github.com/apache/jena into highlighting
commit 452b3838aee02da4983e8df8c6b33c504e7c3f5a
Author: Chris Tomlinson <ct...@moonvine.org>
Date: 2017-11-28T17:57:16Z
added tracing exec, etc for development
commit dbbca204e5a4f7510bb6f667eac7105c28c2ff3e
Author: Chris Tomlinson <ct...@moonvine.org>
Date: 2017-11-28T22:38:05Z
Merge branch 'master' of https://github.com/apache/jena into highlighting
----
---
[GitHub] jena pull request #319: Jena 1441 tracing
Posted by xristy <gi...@git.apache.org>.
Github user xristy commented on a diff in the pull request:
https://github.com/apache/jena/pull/319#discussion_r153838075
--- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
@@ -224,12 +234,16 @@ private QueryIterator resultsToQueryIterator(Binding binding, Node s, Node score
}
private QueryIterator variableSubject(Binding binding, Node s, Node score, Node literal, StrMatch match, ExecutionContext execCxt) {
+ if (log.isTraceEnabled())
+ log.trace("variableSubject: {}", match) ;
--- End diff --
I'm happy to amend by removing the unnecessary `log.isTraceEnabled()`. I was intending to follow the coding practices elsewhere in TextQueryPF and TextIndexLucene, should I amend the unneeded occurrences of `log.isDebugEnabled()` as well?
---
[GitHub] jena issue #319: Jena 1441 tracing
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/319
Logging statements look good to me!
---
[GitHub] jena pull request #319: Jena 1441 tracing
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/319#discussion_r153822758
--- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextIndexLucene.java ---
@@ -204,7 +204,10 @@ public void close() {
@Override public void updateEntity(Entity entity) {
if ( log.isDebugEnabled() )
- log.debug("Update entity: " + entity) ;
+ if (log.isTraceEnabled() && entity != null)
--- End diff --
Better than checking `log.isDebugEnabled()` is to use the idiom `log.debug("Update entity: {}", entity))`, same with other log statements that have guards and are calling `toString` implicitly. Using it for `trace` here makes sense because you aren't just calling `toString` and sadly, SLF4J doesn't yet accept `Supplier`s for messages.
---
[GitHub] jena pull request #319: Jena 1441 tracing
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/319
---
[GitHub] jena pull request #319: Jena 1441 tracing
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/319#discussion_r153861908
--- Diff: jena-text/src/main/java/org/apache/jena/query/text/assembler/TextIndexLuceneAssembler.java ---
@@ -144,6 +144,17 @@ public TextIndex open(Assembler a, Resource root, Mode mode) {
storeValues = svNode.asLiteral().getBoolean();
}
+ // use query cache by default
+ boolean cacheQueries = true;
+ Statement cacheQueriesStatement = root.getProperty(pCacheQueries);
+ if (null != cacheQueriesStatement) {
+ RDFNode cqNode = cacheQueriesStatement.getObject();
+ if (! cqNode.isLiteral()) {
+ throw new TextIndexException("text:cacheQueries property must be a string : " + cqNode);
--- End diff --
This is maybe a little picky, but maybe better: `"text:cacheQueries property must be a boolean`?
---
[GitHub] jena pull request #319: Jena 1441 tracing
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/319#discussion_r153822788
--- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
@@ -224,12 +234,16 @@ private QueryIterator resultsToQueryIterator(Binding binding, Node s, Node score
}
private QueryIterator variableSubject(Binding binding, Node s, Node score, Node literal, StrMatch match, ExecutionContext execCxt) {
+ if (log.isTraceEnabled())
+ log.trace("variableSubject: {}", match) ;
--- End diff --
You don't need to guard with `isTraceEnabled()` if you are using this form of `Logger.trace`.
---
[GitHub] jena pull request #319: Jena 1441 tracing
Posted by xristy <gi...@git.apache.org>.
Github user xristy commented on a diff in the pull request:
https://github.com/apache/jena/pull/319#discussion_r153867385
--- Diff: jena-text/src/main/java/org/apache/jena/query/text/assembler/TextIndexLuceneAssembler.java ---
@@ -144,6 +144,17 @@ public TextIndex open(Assembler a, Resource root, Mode mode) {
storeValues = svNode.asLiteral().getBoolean();
}
+ // use query cache by default
+ boolean cacheQueries = true;
+ Statement cacheQueriesStatement = root.getProperty(pCacheQueries);
+ if (null != cacheQueriesStatement) {
+ RDFNode cqNode = cacheQueriesStatement.getObject();
+ if (! cqNode.isLiteral()) {
+ throw new TextIndexException("text:cacheQueries property must be a string : " + cqNode);
--- End diff --
Thanks. Best to be accurate.
---