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