You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by millerjeff0 <gi...@git.apache.org> on 2018/05/04 19:03:15 UTC
[GitHub] lucene-solr pull request #370: SOLR-12312: Change buf to not always use up 1...
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 <je...@...>
Date: 2018-05-04T19:00:31Z
SOLR-12312: Change buf to not always use up 1 MB
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #370: SOLR-12312: Change buf to not always use up 1...
Posted by millerjeff0 <gi...@git.apache.org>.
Github user millerjeff0 closed the pull request at:
https://github.com/apache/lucene-solr/pull/370
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #370: SOLR-12312: Change buf to not always use up 1...
Posted by millerjeff0 <gi...@git.apache.org>.
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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #370: SOLR-12312: Change buf to not always use up 1...
Posted by dsmiley <gi...@git.apache.org>.
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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #370: SOLR-12312: Change buf to not always use up 1...
Posted by dsmiley <gi...@git.apache.org>.
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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org