dsmiley commented on code in PR #1557:
URL: https://github.com/apache/solr/pull/1557#discussion_r1179707432
##########
solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java:
##########
@@ -372,7 +372,7 @@ public void doc(int docId, StoredFieldVisitor visitor)
throws IOException {
Document cached = doc(docId);
visitFromCached(cached, visitor);
} else {
- searcher.getIndexReader().document(docId, visitor);
+ searcher.getIndexReader().storedFields().document(docId, visitor);
Review Comment:
I think this pattern -- calling storedFields() only to use it for exactly
one document should be banned from any hot code path. And SolrDocumentFetcher
is such. I sympathize that a more elegant solution isn't obvious but we should
do something.
Perhaps we change the contract of SolrIndexSearcher.getDocFetcher (which I
added years ago BTW) to return a new instance (with a storedFields of course)
that is not thread-safe. But nonetheless has all the existing caches of any
other instance pulled from the same searcher. Hmmmm.
Or use an existing well-managed ThreadLocal -- SolrRequestInfo, to hold the
StoredField instance. If it's null (unexpected / non-standard use cases) then
the fallback is grab a storedFields() one-off use. I think this would work
well.
##########
solr/core/src/java/org/apache/solr/handler/admin/IndexSizeEstimator.java:
##########
@@ -384,16 +386,17 @@ private void estimateTermVectors(Map<String, Object>
result) throws IOException
for (LeafReaderContext leafReaderContext : reader.leaves()) {
LeafReader leafReader = leafReaderContext.reader();
Bits liveDocs = leafReader.getLiveDocs();
+ TermVectors termVectors = leafReader.termVectors();
for (int docId = 0; docId < leafReader.maxDoc(); docId += samplingStep) {
if (liveDocs != null && !liveDocs.get(docId)) {
continue;
}
- Fields termVectors = leafReader.getTermVectors(docId);
- if (termVectors == null) {
+ Fields vectors = termVectors.get(docId);
+ if (vectors == null) {
Review Comment:
Why change the variable name? "vectors" is bringing a more ambiguous word
in recent times ("vector search")
##########
solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java:
##########
@@ -251,6 +252,7 @@ public void process(ResponseBuilder rb) throws IOException {
SolrIndexSearcher searcher = rb.req.getSearcher();
IndexReader reader = searcher.getIndexReader();
+ TermVectors tm = reader.termVectors();
Review Comment:
Please improve this variable name, such as use `termVectors`
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
