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/09/09 07:35:38 UTC

[GitHub] [lucene-solr] s1monw opened a new pull request #1847: LUCENE-9514: Include TermVectorsWriter in DWPT accounting

s1monw opened a new pull request #1847:
URL: https://github.com/apache/lucene-solr/pull/1847


   TermVectorsWriter might consume some heap space memory that
   can have a significant impact on decisions made in the IW if
   writers should be stalled or DWPTs should be flushed if memory
   settings are small in IWC and flushes are frequent. This change adds
   RAM accounting to the TermVectorsWriter since it's part of the
   DWPT lifecycle and not just present during flush.
   


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



---------------------------------------------------------------------
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 #1847: LUCENE-9514: Include TermVectorsWriter in DWPT accounting

Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1847:
URL: https://github.com/apache/lucene-solr/pull/1847#discussion_r487721690



##########
File path: lucene/core/src/java/org/apache/lucene/util/Accountable.java
##########
@@ -41,4 +41,8 @@
     return Collections.emptyList();
   }
 
+  /**
+   * An accountable that always returns 0
+   */
+  Accountable NULL_ACCOUNTABLE = () -> 0;

Review comment:
       sure, but do we then initialize all booleans with `= false`? I thinks it's totally obsolet.




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



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


[GitHub] [lucene-solr] uschindler commented on a change in pull request #1847: LUCENE-9514: Include TermVectorsWriter in DWPT accounting

Posted by GitBox <gi...@apache.org>.
uschindler commented on a change in pull request #1847:
URL: https://github.com/apache/lucene-solr/pull/1847#discussion_r487715939



##########
File path: lucene/core/src/java/org/apache/lucene/util/Accountable.java
##########
@@ -41,4 +41,8 @@
     return Collections.emptyList();
   }
 
+  /**
+   * An accountable that always returns 0
+   */
+  Accountable NULL_ACCOUNTABLE = () -> 0;

Review comment:
       To avoid such questions, you can be explicit.




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



---------------------------------------------------------------------
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 #1847: LUCENE-9514: Include TermVectorsWriter in DWPT accounting

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


   


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



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


[GitHub] [lucene-solr] mikemccand commented on a change in pull request #1847: LUCENE-9514: Include TermVectorsWriter in DWPT accounting

Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #1847:
URL: https://github.com/apache/lucene-solr/pull/1847#discussion_r485497049



##########
File path: lucene/core/src/java/org/apache/lucene/util/Accountable.java
##########
@@ -41,4 +41,8 @@
     return Collections.emptyList();
   }
 
+  /**
+   * An accountable that always returns 0
+   */
+  Accountable NULL_ACCOUNTABLE = () -> 0;

Review comment:
       Interface members are always `static final` right?

##########
File path: lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextTermVectorsWriter.java
##########
@@ -193,4 +189,9 @@ private void write(BytesRef bytes) throws IOException {
   private void newLine() throws IOException {
     SimpleTextUtil.writeNewline(out);
   }
+
+  @Override
+  public long ramBytesUsed() {
+    return scratch.get().bytes.length;

Review comment:
       Ha.

##########
File path: lucene/core/src/java/org/apache/lucene/index/StoredFieldsConsumer.java
##########
@@ -42,6 +44,7 @@
   protected void initStoredFieldsWriter() throws IOException {
     if (writer == null) { // TODO can we allocate this in the ctor? we call start document for every doc anyway
       this.writer = codec.storedFieldsFormat().fieldsWriter(directory, info, IOContext.DEFAULT);
+      accountable = writer;

Review comment:
       Hmm what is happening here?  Are we using this new `accountable` member somewhere?  I don't see it in the diffs?  Oh, I see, it is returned in `DefaultIndexingChain.getChildResources()`, ok!  Maybe add comment above its declaration explaining that our parent/owning `DefaultIndexingChain` returns/uses it?




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



---------------------------------------------------------------------
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 #1847: LUCENE-9514: Include TermVectorsWriter in DWPT accounting

Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1847:
URL: https://github.com/apache/lucene-solr/pull/1847#discussion_r487696461



##########
File path: lucene/core/src/java/org/apache/lucene/util/Accountable.java
##########
@@ -41,4 +41,8 @@
     return Collections.emptyList();
   }
 
+  /**
+   * An accountable that always returns 0
+   */
+  Accountable NULL_ACCOUNTABLE = () -> 0;

Review comment:
       yes




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



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