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/30 03:50:13 UTC

[GitHub] [lucene-solr] bringyou opened a new pull request #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates

bringyou opened a new pull request #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates
URL: https://github.com/apache/lucene-solr/pull/1389
 
 
   

----------------------------------------------------------------
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 #1389: LUCENE-9298: Improve RAM accounting in BufferedUpdates when deleted doc IDs and terms are cleared

Posted by GitBox <gi...@apache.org>.
s1monw commented on issue #1389: LUCENE-9298: Improve RAM accounting in BufferedUpdates when deleted doc IDs and terms are cleared
URL: https://github.com/apache/lucene-solr/pull/1389#issuecomment-611976552
 
 
   thanks!!

----------------------------------------------------------------
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] bringyou commented on a change in pull request #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates

Posted by GitBox <gi...@apache.org>.
bringyou commented on a change in pull request #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates
URL: https://github.com/apache/lucene-solr/pull/1389#discussion_r405927650
 
 

 ##########
 File path: lucene/core/src/java/org/apache/lucene/index/BufferedUpdates.java
 ##########
 @@ -176,8 +176,11 @@ void addBinaryUpdate(BinaryDocValuesUpdate update, int docIDUpto) {
   }
 
   void clearDeleteTerms() {
-    deleteTerms.clear();
     numTermDeletes.set(0);
+    deleteTerms.forEach((term, docIDUpto) -> {
 
 Review comment:
   add a counter named `termsBytesUsed`

----------------------------------------------------------------
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] bringyou commented on issue #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates

Posted by GitBox <gi...@apache.org>.
bringyou commented on issue #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates
URL: https://github.com/apache/lucene-solr/pull/1389#issuecomment-611049924
 
 
   > Change looks good to me. Would you mind adding a small test for this issue? Thanks @bringyou!
   
   sorry for the delay~ add a test for `BufferedUpdates` and change a bit more code, please take another look @dnhatn 

----------------------------------------------------------------
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] bringyou commented on issue #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates

Posted by GitBox <gi...@apache.org>.
bringyou commented on issue #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates
URL: https://github.com/apache/lucene-solr/pull/1389#issuecomment-606987371
 
 
   @s1monw sorry to bother, but can you 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

[GitHub] [lucene-solr] s1monw commented on issue #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates

Posted by GitBox <gi...@apache.org>.
s1monw commented on issue #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates
URL: https://github.com/apache/lucene-solr/pull/1389#issuecomment-611393863
 
 
   I ran tests and we are ready to go. Please add a changes entry like this:
   
   ```diff
   diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
   index b5bc71cb759..2daa8f3c71f 100644
   --- a/lucene/CHANGES.txt
   +++ b/lucene/CHANGES.txt
   @@ -238,6 +238,9 @@ Improvements
    * LUCENE-9171: QueryBuilder can now use BoostAttributes on input token streams to selectively
      boost particular terms or synonyms in parsed queries. (Alessandro Benedetti, Alan Woodward)
    
   +* LUCENE-9298: Improve RAM accounting in BufferedUpdates when deleted doc IDs and terms are cleared.
   +  (Your Name, Simon Willnauer)
   +
    Optimizations
    ---------------------
    ```

----------------------------------------------------------------
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 #1389: LUCENE-9298: Improve RAM accounting in BufferedUpdates when deleted doc IDs and terms are cleared

Posted by GitBox <gi...@apache.org>.
s1monw merged pull request #1389: LUCENE-9298: Improve RAM accounting in BufferedUpdates when deleted doc IDs and terms are cleared
URL: https://github.com/apache/lucene-solr/pull/1389
 
 
   

----------------------------------------------------------------
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 a change in pull request #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates

Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates
URL: https://github.com/apache/lucene-solr/pull/1389#discussion_r406017431
 
 

 ##########
 File path: lucene/core/src/java/org/apache/lucene/index/BufferedUpdates.java
 ##########
 @@ -80,6 +80,7 @@ load factor (say 2 * POINTER).  Entry is object w/
 
   private final Counter bytesUsed = Counter.newCounter(true);
   final Counter fieldUpdatesBytesUsed = Counter.newCounter(true);
+  private final Counter termsBytesUsed = Counter.newCounter();
 
 Review comment:
   ```suggestion
     private final Counter termsBytesUsed = Counter.newCounter(true);
   ```
   
   Sorry I looked at it again and we have one case where we use it in a global context in the delete queue so we need the threadsafety. Rest looks great. 

----------------------------------------------------------------
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] bringyou commented on issue #1389: LUCENE-9298: Improve RAM accounting in BufferedUpdates when deleted doc IDs and terms are cleared

Posted by GitBox <gi...@apache.org>.
bringyou commented on issue #1389: LUCENE-9298: Improve RAM accounting in BufferedUpdates when deleted doc IDs and terms are cleared
URL: https://github.com/apache/lucene-solr/pull/1389#issuecomment-611844725
 
 
   @s1monw thanks for the reviews, I add the changelogs and edit issue's title

----------------------------------------------------------------
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 a change in pull request #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates

Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1389: LUCENE-9298: fix clearDeletedDocIds in BufferedUpdates
URL: https://github.com/apache/lucene-solr/pull/1389#discussion_r405796843
 
 

 ##########
 File path: lucene/core/src/java/org/apache/lucene/index/BufferedUpdates.java
 ##########
 @@ -176,8 +176,11 @@ void addBinaryUpdate(BinaryDocValuesUpdate update, int docIDUpto) {
   }
 
   void clearDeleteTerms() {
-    deleteTerms.clear();
     numTermDeletes.set(0);
+    deleteTerms.forEach((term, docIDUpto) -> {
 
 Review comment:
   Instead of counting this here on clear, can we use a second counter for the deleteTerms next to `bytesUsed`? This would be great. It doesn't need to be thread safe IMO

----------------------------------------------------------------
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