lhotari commented on pull request #10944:
URL: https://github.com/apache/pulsar/pull/10944#issuecomment-863490803


   Thanks for the great suggestions @addisonj .
   
   > I wonder if we need a at least a tweak or a different approach... If there 
is any place where we have an arbitrary body that has a redirect in the loop, I 
would be nervous that you would hit this and then get a truncated body on 
redirect.
   
   Yes a different approach would be useful. It seems just wrong to buffer all 
request bodies in memory for the duration of the request handling.
   
   > make it tuneable and the user needs to be aware if they are using HTTP 
endpoints for producing/consuming. Perhaps just setting the default of 5 MB of 
max message size would cover 90% use case? I think this is the minimum
   
   Makes sense. I could add a configuration option. I thought it wouldn't be so 
common to have http requests with large bodies and it would be mainly the 
Pulsar Function jar uploads that could exceed 1MB. I guess with new APIs such 
as PIP-64, 1MB could exceed when sending large messages.
   
   > do something smarter and use an off heap buffer or some smarter allocation 
strategy
   
   Allocating on heap isn't the biggest problem. It's the usage of 
ByteArrayOutputStream which uses a single byte array. 
   I'm the author of StreamByteBuffer/StreamCharBuffer classes which are used 
in Grails, Gradle and MyFaces for smarter buffering. Essentially the key idea 
is to use a linked list of smaller byte arrays. For example [StreamByteBuffer 
in 
Gradle](https://github.com/gradle/gradle/blob/master/subprojects/base-services/src/main/java/org/gradle/internal/io/StreamByteBuffer.java)
 / [StreamByteBuffer in 
Grails](https://github.com/grails/grails-core/blob/master/grails-encoder/src/main/groovy/org/grails/buffer/StreamByteBuffer.java)
 shows this. Both versions are covered with Apache license and I'm the original 
author so there shouldn't be a problem in reusing some part of those classes in 
a StreamByteBuffer for Pulsar. The simplest version wouldn't require a String 
conversion which is most of the complexity in the above examples.
   
   > make the proxy smarter and if it is a request that has a post body and may 
be redirected we do a lookup and then always hit the correct broker. This might 
be a longer term effort
   
   I think this would be a good solution.
   
    
   
   
   
   
   
   
   
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to