dsmiley commented on code in PR #2021:
URL: https://github.com/apache/solr/pull/2021#discussion_r1408490861


##########
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java:
##########
@@ -507,29 +504,19 @@ private boolean doVersionAdd(
                     lastVersion);
                 return true;
               } else {
-                // We're good, we should apply this update. First, update the 
bucket's highest.
-                if (bucketVersion != 0 && bucketVersion < versionOnUpdate) {
-                  bucket.updateHighest(versionOnUpdate);
-                }
+                // We're good, we should apply this update.
               }
             }
           } else {
             // if we aren't the leader, then we need to check that updates 
were not re-ordered
-            if (bucketVersion != 0 && bucketVersion < versionOnUpdate) {
-              // we're OK... this update has a version higher than anything 
we've seen
-              // in this bucket so far, so we know that no reordering has yet 
occurred.
-              bucket.updateHighest(versionOnUpdate);
-            } else {
-              // there have been updates higher than the current update. we 
need to check
-              // the specific version for this id.
-              Long lastVersion = vinfo.lookupVersion(cmd.getIndexedId());

Review Comment:
   I suppose the current logic is trying to avoid this very call here -- 
`vinfo.lookupVersion` which probably isn't all that expensive in the context of 
indexing a doc but it's something.  It must get a "realTimeSearcher", which 
usually can use the existing one (assume a sane commit strategy; not per doc).  
It looks up the doc ID by a term dictionary lookup of the uniqueKey value, then 
with that goes to resolve the version via numeric docValues retrieval (if the 
doc actually exists, i.e. is an update).
   
   I'm still +1 but just identifying trade-offs made in the past; perhaps 
prematurely with a high complexity price.
   
   And expect a follow-up PR from me to eliminate the "buckets" altogether :-)



-- 
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: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to