[GitHub] lucene-solr pull request #370: SOLR-12312: Change buf to not always use up 1...

[millerjeff0]

Github user millerjeff0 closed the pull request at:

https://github.com/apache/lucene-solr/pull/370


---

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

[GitHub] lucene-solr pull request #370: SOLR-12312: Change buf to not always use up 1...

[millerjeff0]

Github user millerjeff0 commented on a diff in the pull request:

https://github.com/apache/lucene-solr/pull/370#discussion_r186210634
  
--- Diff: solr/core/src/java/org/apache/solr/handler/IndexFetcher.java ---
@@ -1629,8 +1630,6 @@ private int fetchPackets(FastInputStream fis) throws 
Exception {
 LOG.warn("No content received for file: {}", fileName);
 return NO_CONTENT;
   }
-  if (buf.length < packetSize)
--- End diff --

Hmm..ok that makes sense as a safety mechanism. It's effectively done by 
settings to PACKET_SZ anyways.


---

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

[GitHub] lucene-solr pull request #370: SOLR-12312: Change buf to not always use up 1...

[dsmiley]

Github user dsmiley commented on a diff in the pull request:

https://github.com/apache/lucene-solr/pull/370#discussion_r186205859
  
--- Diff: solr/core/src/java/org/apache/solr/handler/IndexFetcher.java ---
@@ -1629,8 +1630,6 @@ private int fetchPackets(FastInputStream fis) throws 
Exception {
 LOG.warn("No content received for file: {}", fileName);
 return NO_CONTENT;
   }
-  if (buf.length < packetSize)
--- End diff --

I appreciate that this check may be needless now but it feels awfully 
presumptuous to me that this could never happen.  Like what if PACKET_SZ 
changes some day and someone is upgrading their Solr cluster with mixed 
versions at the same time and a larger packet is sent?  Even an assert feels to 
strict.  It could happen but it's not expected to.  So leave it and add a 
simple one-liner comment that in practice this won't happen but we adjust in 
case it does.


---

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

[GitHub] lucene-solr pull request #370: SOLR-12312: Change buf to not always use up 1...

[dsmiley]

Github user dsmiley commented on a diff in the pull request:

https://github.com/apache/lucene-solr/pull/370#discussion_r186196579
  
--- Diff: solr/core/src/java/org/apache/solr/handler/IndexFetcher.java ---
@@ -1549,6 +1549,8 @@ public ReplicationHandlerException(String message) {
   this.file = file;
   this.fileName = (String) fileDetails.get(NAME);
   this.size = (Long) fileDetails.get(SIZE);
+  //Max buf of 1 MB, but we should save memory if the file size is 
smaller
+  buf = this.size < 1048576 ? new byte[(int) this.size] : new 
byte[1024 * 1024];
--- End diff --

I observe 1048576 is 1024*1024.
Lets define a constant DEFAULT_BUF_SIZE = 1024 * 1024  (or perhaps call 
MAX_BUF_SIZE?) and then use this constant twice here.  And you can simply use 
`Math.min(this.size(), DEFAULT_BUF_SIZE)`.  No need for any comment; it's plain 
enough (IMO).


---

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

[GitHub] lucene-solr pull request #370: SOLR-12312: Change buf to not always use up 1...

[millerjeff0]

GitHub user millerjeff0 opened a pull request:

https://github.com/apache/lucene-solr/pull/370

SOLR-12312: Change buf to not always use up 1 MB

A lot of replicated files aren't 1 MB in size, this causes a lot of heap 
space waste when we create this for every file, instead create buffer based on 
the file size with a max of 1 MB

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/millerjeff0/lucene-solr SOLR-12312

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/lucene-solr/pull/370.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #370


commit 3499667b1b88bef207ab5825963e54db62d5eb2c
Author: Jeff 
Date:   2018-05-04T19:00:31Z

SOLR-12312: Change buf to not always use up 1 MB




---

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