You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "stillalex (via GitHub)" <gi...@apache.org> on 2023/07/17 15:45:19 UTC

[GitHub] [solr] stillalex opened a new pull request, #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

stillalex opened a new pull request, #1789:
URL: https://github.com/apache/solr/pull/1789

   https://issues.apache.org/jira/browse/SOLR-16265
   
   <!--
   _(If you are a project committer then you may remove some/all of the following template.)_
   
   Before creating a pull request, please file an issue in the ASF Jira system for Solr:
   
   * https://issues.apache.org/jira/projects/SOLR
   
   For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA. 
   To create a Jira issue, you will need to create an account there first.
   
   The title of the PR should reference the Jira issue number in the form:
   
   * SOLR-####: <short description of problem or changes>
   
   SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
   
   Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
   
   
   # Description
   
   Please provide a short description of the changes you're making with this pull request.
   
   # Solution
   
   Please provide a short description of the approach taken to implement your solution.
   
   # Tests
   
   Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
   - [ ] I have created a Jira issue and added the issue ID to my pull request title.
   - [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [ ] I have developed this patch against the `main` branch.
   - [ ] I have run `./gradlew check`.
   - [ ] I have added tests for my changes.
   - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
   


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] risdenk commented on a diff in pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1789:
URL: https://github.com/apache/solr/pull/1789#discussion_r1283640058


##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -755,11 +762,12 @@ private Request fillContentStream(
           content.addFilePart(
               name,
               contentStream.getName(),
-              new InputStreamContentProvider(contentStream.getStream()),
+              new InputStreamRequestContent(contentStream.getStream()),
               fields);
         }
       }
-      req.content(content);
+      content.close();

Review Comment:
   Do we have to worry about closing this if there is an exception? not sure if `MultiPartRequestContent` can do try w/ resources?



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] stillalex merged pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex merged PR #1789:
URL: https://github.com/apache/solr/pull/1789


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] stillalex commented on pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1789:
URL: https://github.com/apache/solr/pull/1789#issuecomment-1638426454

   Attaching memory footprint of client side during indexing benchmark. While perf numbers are very similar, memory footprint definitely sees a drop.
   
   * main branch
   
   <img width="1449" alt="bench-http2client-main" src="https://github.com/apache/solr/assets/61026/58c5d718-94a4-454a-ac09-470d5fedacc5">
   
   * proposed patch
   
   <img width="1449" alt="bench-http2client-patch" src="https://github.com/apache/solr/assets/61026/ef9e1547-4cb7-467c-ad1c-f0e4255eee9c">
   


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] stillalex commented on pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1789:
URL: https://github.com/apache/solr/pull/1789#issuecomment-1668368156

   build failed due to some timeout issue. is there anyway to re-trigger?


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] stillalex commented on pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1789:
URL: https://github.com/apache/solr/pull/1789#issuecomment-1668373130

   will push changes.txt to trigger a new build (I needed to do that anyway)


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] stillalex commented on a diff in pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1789:
URL: https://github.com/apache/solr/pull/1789#discussion_r1286170365


##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -438,81 +436,78 @@ public void send(OutStream outStream, SolrRequest<?> req, String collection) thr
   private static final Cancellable FAILED_MAKING_REQUEST_CANCELLABLE = () -> {};
 
   public Cancellable asyncRequest(
-      SolrRequest<?> solrRequest,
-      String collection,
-      AsyncListener<NamedList<Object>> asyncListener) {
+      SolrRequest<?> solrReq, String collection, AsyncListener<NamedList<Object>> asyncListener) {
+    MDCCopyHelper mdcCopyHelper = new MDCCopyHelper();
+    SolrRequest<?> solrRequest = unwrapV2Request(solrReq);
+
     Request req;
     try {
-      req = makeRequest(solrRequest, collection);
-    } catch (SolrServerException | IOException e) {
-      asyncListener.onFailure(e);
-      return FAILED_MAKING_REQUEST_CANCELLABLE;
-    }
-    final ResponseParser parser =
-        solrRequest.getResponseParser() == null ? this.parser : solrRequest.getResponseParser();
-    MDCCopyHelper mdcCopyHelper = new MDCCopyHelper();
-    req.onRequestQueued(asyncTracker.queuedListener)
-        .onComplete(asyncTracker.completeListener)
-        .send(
-            new InputStreamResponseListener() {
-              @Override
-              public void onHeaders(Response response) {
-                super.onHeaders(response);
-                InputStreamResponseListener listener = this;
-                executor.execute(
-                    () -> {
-                      InputStream is = listener.getInputStream();
-                      assert ObjectReleaseTracker.track(is);
-                      try {
-                        NamedList<Object> body =
-                            processErrorsAndResponse(
-                                solrRequest, parser, response, is, req.getURI().toString());
+      String url = getRequestPath(solrRequest, collection);
+      InputStreamResponseListener listener =
+          new InputStreamResponseListener() {
+            @Override
+            public void onHeaders(Response response) {
+              super.onHeaders(response);
+              InputStreamResponseListener listener = this;

Review Comment:
   does not seem to be needed. weird, removed it now



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] stillalex commented on a diff in pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1789:
URL: https://github.com/apache/solr/pull/1789#discussion_r1283685478


##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -755,11 +762,12 @@ private Request fillContentStream(
           content.addFilePart(
               name,
               contentStream.getName(),
-              new InputStreamContentProvider(contentStream.getStream()),
+              new InputStreamRequestContent(contentStream.getStream()),
               fields);
         }
       }
-      req.content(content);
+      content.close();

Review Comment:
   good idea! seems it does so I will give it a go.



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] sonatype-lift[bot] commented on pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "sonatype-lift[bot] (via GitHub)" <gi...@apache.org>.
sonatype-lift[bot] commented on PR #1789:
URL: https://github.com/apache/solr/pull/1789#issuecomment-1638406118

   ## Sonatype Lift is retiring
   Sonatype Lift will be retiring on <b>Sep 12, 2023</b>, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
   We are extremely grateful and thank you for your support over the years.
   
   <b>[📖 Read about the impacts and timeline](https://www.sonatype.com/lift-retirement)</b>


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] stillalex commented on pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1789:
URL: https://github.com/apache/solr/pull/1789#issuecomment-1666085178

   thank you @risdenk and @markrmiller for the reviews. I have added one more change which was a bit awkward. I needed the request url for the exception message inside the async listener, so I moved some methods around to build the url first. it's the least bad solution I could come up with. this doesn't change much of the PR, so I will go ahead and try to push this in once the builds complete.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] stillalex commented on a diff in pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1789:
URL: https://github.com/apache/solr/pull/1789#discussion_r1286170112


##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -659,62 +633,97 @@ private Request createRequest(SolrRequest<?> solrRequest, String collection)
         basePath = serverBaseUrl + "/____v2";
       }
     }
+    String path = requestWriter.getPath(solrRequest);
+    if (path == null || !path.startsWith("/")) {
+      path = DEFAULT_PATH;
+    }
+
+    return basePath + path;
+  }
+
+  private Request makeRequestAndSend(
+      SolrRequest<?> solrRequest, String url, InputStreamResponseListener listener, boolean isAsync)
+      throws IOException, SolrServerException {
+
+    // TODO add invariantParams support
+    ResponseParser parser =
+        solrRequest.getResponseParser() == null ? this.parser : solrRequest.getResponseParser();
+
+    // The parser 'wt=' and 'version=' params are used instead of the original
+    // params
+    ModifiableSolrParams wparams = new ModifiableSolrParams(solrRequest.getParams());
+    wparams.set(CommonParams.WT, parser.getWriterType());
+    wparams.set(CommonParams.VERSION, parser.getVersion());
 
     if (SolrRequest.METHOD.GET == solrRequest.getMethod()) {
-      if (streams != null || contentWriter != null) {
+      RequestWriter.ContentWriter contentWriter = requestWriter.getContentWriter(solrRequest);
+      Collection<ContentStream> streams =
+          contentWriter == null ? requestWriter.getContentStreams(solrRequest) : null;
+      if (contentWriter != null || streams != null) {
         throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "GET can't send streams!");
       }
-
-      return httpClient
-          .newRequest(basePath + path + wparams.toQueryString())
-          .method(HttpMethod.GET);
+      var r = httpClient.newRequest(url + wparams.toQueryString()).method(HttpMethod.GET);
+      decorateRequest(r, solrRequest, isAsync);
+      r.send(listener);
+      return r;
     }
 
     if (SolrRequest.METHOD.DELETE == solrRequest.getMethod()) {
-      return httpClient
-          .newRequest(basePath + path + wparams.toQueryString())
-          .method(HttpMethod.DELETE);
+      var r = httpClient.newRequest(url + wparams.toQueryString()).method(HttpMethod.DELETE);
+      decorateRequest(r, solrRequest, isAsync);
+      r.send(listener);
+      return r;
     }
 
     if (SolrRequest.METHOD.POST == solrRequest.getMethod()
         || SolrRequest.METHOD.PUT == solrRequest.getMethod()) {
+      RequestWriter.ContentWriter contentWriter = requestWriter.getContentWriter(solrRequest);
+      Collection<ContentStream> streams =
+          contentWriter == null ? requestWriter.getContentStreams(solrRequest) : null;
 
-      String url = basePath + path;
-      boolean hasNullStreamName = false;
+      boolean isMultipart = false;
       if (streams != null) {
+        boolean hasNullStreamName = false;
         hasNullStreamName = streams.stream().anyMatch(cs -> cs.getName() == null);
+        isMultipart = !hasNullStreamName && streams.size() > 1;
       }
 
-      boolean isMultipart = streams != null && streams.size() > 1 && !hasNullStreamName;
-
       HttpMethod method =
           SolrRequest.METHOD.POST == solrRequest.getMethod() ? HttpMethod.POST : HttpMethod.PUT;
 
       if (contentWriter != null) {
-        Request req = httpClient.newRequest(url + wparams.toQueryString()).method(method);
-        Utils.BAOS baos = new Utils.BAOS();
-        contentWriter.write(baos);
-
-        // SOLR-16265: TODO reduce memory usage
-        return req.content(
-            // We're throwing this BAOS away, so no need to copy the byte[], just use the raw buf
-            new ByteBufferContentProvider(
-                contentWriter.getContentType(), ByteBuffer.wrap(baos.getbuf(), 0, baos.size())));
+        OutputStreamRequestContent content =

Review Comment:
   agreed. I added a few of those below, this one got away. replaced it with latest commit



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] stillalex commented on a diff in pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1789:
URL: https://github.com/apache/solr/pull/1789#discussion_r1283691001


##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -755,11 +762,12 @@ private Request fillContentStream(
           content.addFilePart(
               name,
               contentStream.getName(),
-              new InputStreamContentProvider(contentStream.getStream()),
+              new InputStreamRequestContent(contentStream.getStream()),
               fields);
         }
       }
-      req.content(content);
+      content.close();

Review Comment:
   change is in!



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] epugh commented on pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1789:
URL: https://github.com/apache/solr/pull/1789#issuecomment-1664574973

   I wish we had more confidence in our tests (benchmark and unit) that a change like this that passed the tests could be assumed to correct ;-).   I like anything that removes deprecated code!


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] stillalex commented on pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1789:
URL: https://github.com/apache/solr/pull/1789#issuecomment-1664610985

   > I wish we had more confidence in our tests (benchmark and unit) that a change like this that passed the tests could be assumed to correct ;-). I like anything that removes deprecated code!
   
   oh yeah, I completely agree. how can we increase that confidence? I feel like there are quite a few tests already (running 'gradlew check' is already a lengthy affair)
   
   


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [solr] dsmiley commented on a diff in pull request #1789: SOLR-16265 reduce memory usage of ContentWriter based requests in Http2SolrClient

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1789:
URL: https://github.com/apache/solr/pull/1789#discussion_r1284960797


##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -659,62 +633,97 @@ private Request createRequest(SolrRequest<?> solrRequest, String collection)
         basePath = serverBaseUrl + "/____v2";
       }
     }
+    String path = requestWriter.getPath(solrRequest);
+    if (path == null || !path.startsWith("/")) {
+      path = DEFAULT_PATH;
+    }
+
+    return basePath + path;
+  }
+
+  private Request makeRequestAndSend(
+      SolrRequest<?> solrRequest, String url, InputStreamResponseListener listener, boolean isAsync)
+      throws IOException, SolrServerException {
+
+    // TODO add invariantParams support
+    ResponseParser parser =
+        solrRequest.getResponseParser() == null ? this.parser : solrRequest.getResponseParser();
+
+    // The parser 'wt=' and 'version=' params are used instead of the original
+    // params
+    ModifiableSolrParams wparams = new ModifiableSolrParams(solrRequest.getParams());
+    wparams.set(CommonParams.WT, parser.getWriterType());
+    wparams.set(CommonParams.VERSION, parser.getVersion());
 
     if (SolrRequest.METHOD.GET == solrRequest.getMethod()) {
-      if (streams != null || contentWriter != null) {
+      RequestWriter.ContentWriter contentWriter = requestWriter.getContentWriter(solrRequest);
+      Collection<ContentStream> streams =
+          contentWriter == null ? requestWriter.getContentStreams(solrRequest) : null;
+      if (contentWriter != null || streams != null) {
         throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "GET can't send streams!");
       }
-
-      return httpClient
-          .newRequest(basePath + path + wparams.toQueryString())
-          .method(HttpMethod.GET);
+      var r = httpClient.newRequest(url + wparams.toQueryString()).method(HttpMethod.GET);
+      decorateRequest(r, solrRequest, isAsync);
+      r.send(listener);
+      return r;
     }
 
     if (SolrRequest.METHOD.DELETE == solrRequest.getMethod()) {
-      return httpClient
-          .newRequest(basePath + path + wparams.toQueryString())
-          .method(HttpMethod.DELETE);
+      var r = httpClient.newRequest(url + wparams.toQueryString()).method(HttpMethod.DELETE);
+      decorateRequest(r, solrRequest, isAsync);
+      r.send(listener);
+      return r;
     }
 
     if (SolrRequest.METHOD.POST == solrRequest.getMethod()
         || SolrRequest.METHOD.PUT == solrRequest.getMethod()) {
+      RequestWriter.ContentWriter contentWriter = requestWriter.getContentWriter(solrRequest);
+      Collection<ContentStream> streams =
+          contentWriter == null ? requestWriter.getContentStreams(solrRequest) : null;
 
-      String url = basePath + path;
-      boolean hasNullStreamName = false;
+      boolean isMultipart = false;
       if (streams != null) {
+        boolean hasNullStreamName = false;
         hasNullStreamName = streams.stream().anyMatch(cs -> cs.getName() == null);
+        isMultipart = !hasNullStreamName && streams.size() > 1;
       }
 
-      boolean isMultipart = streams != null && streams.size() > 1 && !hasNullStreamName;
-
       HttpMethod method =
           SolrRequest.METHOD.POST == solrRequest.getMethod() ? HttpMethod.POST : HttpMethod.PUT;
 
       if (contentWriter != null) {
-        Request req = httpClient.newRequest(url + wparams.toQueryString()).method(method);
-        Utils.BAOS baos = new Utils.BAOS();
-        contentWriter.write(baos);
-
-        // SOLR-16265: TODO reduce memory usage
-        return req.content(
-            // We're throwing this BAOS away, so no need to copy the byte[], just use the raw buf
-            new ByteBufferContentProvider(
-                contentWriter.getContentType(), ByteBuffer.wrap(baos.getbuf(), 0, baos.size())));
+        OutputStreamRequestContent content =

Review Comment:
   Just want to mention that cases like this are perfect for "var".  For next time...



##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -438,81 +436,78 @@ public void send(OutStream outStream, SolrRequest<?> req, String collection) thr
   private static final Cancellable FAILED_MAKING_REQUEST_CANCELLABLE = () -> {};
 
   public Cancellable asyncRequest(
-      SolrRequest<?> solrRequest,
-      String collection,
-      AsyncListener<NamedList<Object>> asyncListener) {
+      SolrRequest<?> solrReq, String collection, AsyncListener<NamedList<Object>> asyncListener) {
+    MDCCopyHelper mdcCopyHelper = new MDCCopyHelper();
+    SolrRequest<?> solrRequest = unwrapV2Request(solrReq);
+
     Request req;
     try {
-      req = makeRequest(solrRequest, collection);
-    } catch (SolrServerException | IOException e) {
-      asyncListener.onFailure(e);
-      return FAILED_MAKING_REQUEST_CANCELLABLE;
-    }
-    final ResponseParser parser =
-        solrRequest.getResponseParser() == null ? this.parser : solrRequest.getResponseParser();
-    MDCCopyHelper mdcCopyHelper = new MDCCopyHelper();
-    req.onRequestQueued(asyncTracker.queuedListener)
-        .onComplete(asyncTracker.completeListener)
-        .send(
-            new InputStreamResponseListener() {
-              @Override
-              public void onHeaders(Response response) {
-                super.onHeaders(response);
-                InputStreamResponseListener listener = this;
-                executor.execute(
-                    () -> {
-                      InputStream is = listener.getInputStream();
-                      assert ObjectReleaseTracker.track(is);
-                      try {
-                        NamedList<Object> body =
-                            processErrorsAndResponse(
-                                solrRequest, parser, response, is, req.getURI().toString());
+      String url = getRequestPath(solrRequest, collection);
+      InputStreamResponseListener listener =
+          new InputStreamResponseListener() {
+            @Override
+            public void onHeaders(Response response) {
+              super.onHeaders(response);
+              InputStreamResponseListener listener = this;

Review Comment:
   must this be defined a second time?



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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