jdyer1 commented on code in PR #2259:
URL: https://github.com/apache/solr/pull/2259#discussion_r1506446418
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrJdkClient.java:
##########
@@ -188,33 +210,52 @@ private HttpResponse<InputStream> doPutOrPost(
}
HttpRequest.BodyPublisher bodyPublisher;
-
+ final PipedOutputStream source;
if (contentWriter != null) {
- // TODO: There is likely a more memory-efficient way to do this!
- ByteArrayOutputStream baos = new ByteArrayOutputStream();
- contentWriter.write(baos);
- byte[] bytes = baos.toByteArray();
- bodyPublisher = HttpRequest.BodyPublishers.ofByteArray(bytes);
+ source = new PipedOutputStream();
+ PipedInputStream sink = new PipedInputStream(source);
+ bodyPublisher = HttpRequest.BodyPublishers.ofInputStream(() -> sink);
+ // NOCOMMIT - this does not work. The call to "client.send" sometimes
will throw an
+ // IOException
+ // because our thread closes the outputstream too early. But if we wait
to close it until
Review Comment:
I removed this NOCOMMIT and removed the unnecessary second close on the
OutputStream. I also changed it to hold onto the Future so as to cancel on
exception. I also made various other improvements.
The OutputStream was being closed early, but only on non-secure http2
requests. If using TLS or forcing Http1.1, it always worked. The problem was
that with the first request, the JDK Http Client sends it as Http1.1 and
requests an upgrade. When that first request happens to be a POST with a
chunked body, it evidently sends the body with the upgrade request. Our Jetty
server, however, does not allow a body with an upgrade request, so it replies
with a GOAWAY frame. The workaround I found is to issue a "ping" request prior
to the first real one. This lets the JDK Client know if our server supports
Http2 in advance and upgrade requests are avoided after that.
I have two regrets with this round of changes: first, I cannot figure out a
reliable way to write a unit test for the error handling when it calls "cancel"
on the Future. Second, I am calling "ping" in the constructor, which I felt
was a lesser evil than having to check a volatile boolean with each request to
see if "ping" was called yet.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
