You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/03/13 20:04:33 UTC

[GitHub] [lucene-solr] s1monw opened a new pull request #1350: Cleanup DWPT for readability

s1monw opened a new pull request #1350: Cleanup DWPT for readability
URL: https://github.com/apache/lucene-solr/pull/1350
 
 
   DWPT had some complicated logic to account for failures etc.
   This change cleans up this logic and simplifies the document processing
   loop.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dnhatn commented on a change in pull request #1350: Cleanup DWPT for readability

Posted by GitBox <gi...@apache.org>.
dnhatn commented on a change in pull request #1350: Cleanup DWPT for readability
URL: https://github.com/apache/lucene-solr/pull/1350#discussion_r392589684
 
 

 ##########
 File path: lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java
 ##########
 @@ -318,8 +301,10 @@ private long finishDocuments(DocumentsWriterDeleteQueue.Node<?> deleteNode, int
 
   // Buffer a specific docID for deletion. Currently only
   // used when we hit an exception when adding a document
-  void deleteDocID(int docIDUpto) {
-    pendingUpdates.addDocID(docIDUpto);
+  private void deleteLastDocs(int docCount) {
 
 Review comment:
   Can you update the javadocs (it's a bit stale).

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dnhatn commented on a change in pull request #1350: Cleanup DWPT for readability

Posted by GitBox <gi...@apache.org>.
dnhatn commented on a change in pull request #1350: Cleanup DWPT for readability
URL: https://github.com/apache/lucene-solr/pull/1350#discussion_r392589649
 
 

 ##########
 File path: lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java
 ##########
 @@ -249,34 +247,19 @@ public long updateDocuments(Iterable<? extends Iterable<? extends IndexableField
           reserveOneDoc();
           docState.doc = doc;
           docState.docID = numDocsInRAM;
-          docCount++;
-
-          boolean success = false;
           try {
             consumer.processDocument();
-            success = true;
           } finally {
-            if (!success) {
-              // Incr here because finishDocument will not
-              // be called (because an exc is being thrown):
-              numDocsInRAM++;
-            }
+            numDocsInRAM++; // we count the doc anyway even in the case of an exception
           }
-
-          numDocsInRAM++;
         }
         allDocsIndexed = true;
-        return finishDocuments(deleteNode, docCount);
+        return finishDocuments(deleteNode, docsInRamBefore);
       } finally {
         if (!allDocsIndexed && !aborted) {
           // the iterator threw an exception that is not aborting
           // go and mark all docs from this block as deleted
-          int docID = numDocsInRAM - 1;
-          final int endDocID = docID - docCount;
-          while (docID > endDocID) {
-            deleteDocID(docID);
-            docID--;
-          }
+          deleteLastDocs(numDocsInRAM-docsInRamBefore);
 
 Review comment:
   nit:  add spaces around `-`

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw merged pull request #1350: Cleanup DWPT for readability

Posted by GitBox <gi...@apache.org>.
s1monw merged pull request #1350: Cleanup DWPT for readability
URL: https://github.com/apache/lucene-solr/pull/1350
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dnhatn commented on a change in pull request #1350: Cleanup DWPT for readability

Posted by GitBox <gi...@apache.org>.
dnhatn commented on a change in pull request #1350: Cleanup DWPT for readability
URL: https://github.com/apache/lucene-solr/pull/1350#discussion_r392589684
 
 

 ##########
 File path: lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java
 ##########
 @@ -318,8 +301,10 @@ private long finishDocuments(DocumentsWriterDeleteQueue.Node<?> deleteNode, int
 
   // Buffer a specific docID for deletion. Currently only
   // used when we hit an exception when adding a document
-  void deleteDocID(int docIDUpto) {
-    pendingUpdates.addDocID(docIDUpto);
+  private void deleteLastDocs(int docCount) {
 
 Review comment:
   Can you update the javadocs (it's a bit stale: `a specific docID`).

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw commented on issue #1350: Cleanup DWPT for readability

Posted by GitBox <gi...@apache.org>.
s1monw commented on issue #1350: Cleanup DWPT for readability
URL: https://github.com/apache/lucene-solr/pull/1350#issuecomment-598890923
 
 
   @dnhatn wanna take a look?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org