Based on the private discussion with Yubiao, here is the current update:
Currently, we can get the init parameter `requestBufferSize ` from the
ServletConfig. We need to investigate this configuring method to avoid
the potential conflict issue. And we may not need to expose
`httpClientRequestBufferSize` in the proxy configuration.
@Yubiao, Please correct me or provide more context if I am wrong.
Thanks,
Zike Yang
Zike Yang
On Tue, Mar 21, 2023 at 2:50 PM Yubiao Feng
<yubiao.f...@streamnative.io.invalid> wrote:
>
> Hi Zike
>
> > Exposing `httpClientRequestBufferSize` to help users improve the
> > forwarding performance.
>
> It's just an added benefit.
>
> The primary goal is to solve the long HTTP URL.
> Since Pulsar Proxy forwards the request to Pulsar Server, it will throw
> BufferOverflowException if the length of the URL is lang than 4096( default
> value of `httpClientRequestBufferSize` ), because the URI and all the HTTP
> headers will share the same ByteBuf, such as this:
>
> There has an HTTP request like this:
>
> ```
> GET /admin/v2/public/default/tp/stats HTTP/1.1
> Host: 127.0.0.1
> Accept: application/json
> ....
> ```
>
> The internal client of Pulsar Proxy will work like this:
>
> ```
> ByteBuf buf = allocate( config.httpClientRequestBufferSize )
> buf.write("GET /admin/v2/public/default/tp/stats HTTP/1.1");
> buf.write("Host: 127.0.0.1");
> buf.write("Accept: application/json")
> ...
> buf.flush()
> ```
>
> If the data is larger than the ByteBuf, we will get a
> BufferOverflowException, and users will get an error with the reason
> "Request header too large."
>
> https://github.com/eclipse/jetty.project/blob/574ad3b4daacf0d992f40ab780f2425ecbbac7bb/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java#L178-L183
>
>
> Thanks
> Yubiao Feng
>
> On Tue, Mar 21, 2023 at 11:16 AM Zike Yang <z...@apache.org> wrote:
>
> > Hi, Yubiao
> >
> > Thanks for your explanation. So from my understanding, there are
> > actually two different goals in this PIP:
> > * Exposing `httpMaxRequestHeaderSize` to solve the long name issue:
> > https://lists.apache.org/thread/q1m23ckyy10wvtzy65v8bwqwnh7r0gc8
> > * Exposing `httpClientRequestBufferSize` to help users improve the
> > forwarding performance.
> >
> > I'm +1 if my understanding is correct.
> >
> > Thanks,
> > Zike Yang
> >
> > On Mon, Mar 20, 2023 at 10:07 PM Yubiao Feng
> > <yubiao.f...@streamnative.io.invalid> wrote:
> > >
> > > Hi Zike
> > >
> > > > Is it worth exposing `httpClientRequestBufferSize` to the proxy user?
> > > > Seems exposing `httpMaxRequestHeaderSize ` is enough to solve this
> > > > issue. We can set the buffer size in the proxy code based on the value
> > > > of `httpMaxRequestHeaderSize `. Is there any case that we need to
> > > > expose `httpClientRequestBufferSize`?
> > >
> > > 1. These two configurations work on two very different components: The
> > > config "httpMaxRequestHeaderSize" is used for the internal server of
> > > Pulsar-Proxy, and the config "httpClientRequestBufferSize" is used for
> > the
> > > internal HTTP client of the Pulsar-Proxy.
> > >
> > > 2. Users can modify the configure `httpClientRequestBufferSize` to adjust
> > > the buffer of proxy forwarding requests to improve forwarding
> > performance.
> > >
> > > So it's better to have two separate configurations
> > >
> > > Thanks
> > > Yubiao Feng
> > >
> > >
> > >
> > > On Mon, Mar 20, 2023 at 6:36 PM Zike Yang <z...@apache.org> wrote:
> > >
> > > > > private int httpClientRequestBufferSize =
> > httpMaxRequestHeaderSize;
> > > >
> > > > Is it worth exposing `httpClientRequestBufferSize` to the proxy user?
> > > > Seems exposing `httpMaxRequestHeaderSize ` is enough to solve this
> > > > issue. We can set the buffer size in the proxy code based on the value
> > > > of `httpMaxRequestHeaderSize `. Is there any case that we need to
> > > > expose `httpClientRequestBufferSize`?
> > > >
> > > > Thanks,
> > > > Zike Yang
> > > >
> > > > On Fri, Mar 17, 2023 at 7:31 PM 丛搏 <bog...@apache.org> wrote:
> > > > >
> > > > > +1
> > > > >
> > > > > Hi, Yubiao :
> > > > >
> > > > > Thanks for your explanation. That makes sense to me.
> > > > >
> > > > > Thanks,
> > > > > Bo
> > > > >
> > > > >
> > > > > Yubiao Feng <yubiao.f...@streamnative.io.invalid> 于2023年3月17日周五
> > 16:29写道：
> > > > > >
> > > > > > Hi Bo
> > > > > >
> > > > > > > I have a question, why we need `httpClientRequestBufferSize ` in
> > > > > > > proxy, can you explain in detail?
> > > > > >
> > > > > > Since The Pulsar-Proxy uses the tool `jetty-client` to forward HTTP
> > > > > > requests from users to The Pulsar-Broker, if the proxy receives a
> > > > request
> > > > > > like this:
> > > > > >
> > > > > > ```
> > > > > > GET /admin/v2/public/default/tp.....long......long..../stats
> > HTTP/1.1
> > > > > > ```
> > > > > >
> > > > > > The internal client with forward this request like this:
> > > > > >
> > > > > > ```
> > > > > > ByteBuf buf = allocate( config.httpClientRequestBufferSize )
> > > > > > buf.write(requestLine); // (Highlight) we will get
> > > > > > a BufferOverflowException if the request line is too long.
> > > > > > ```
> > > > > >
> > > > > > Therefore, in addition to ensuring that the proxy server can
> > receive a
> > > > long
> > > > > > request line, the internal client must also process a long request
> > > > line.
> > > > > > And this problem can be solved by making configuration
> > > > > > `httpClientRequestBufferSize` configurable.
> > > > > >
> > > > > >
> > > > > > Thanks
> > > > > > Yubiao Feng
> > > > > >
> > > > > >
> > > > > > On Thu, Mar 16, 2023 at 8:12 PM 丛搏 <bog...@apache.org> wrote:
> > > > > >
> > > > > > > hi yubiao :
> > > > > > >
> > > > > > > I have a question, why we need `httpClientRequestBufferSize ` in
> > > > > > > proxy, can you explain in detail?
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Bo
> > > > > > >
> > > > > > > Yubiao Feng <yubiao.f...@streamnative.io.invalid> 于2023年3月16日周四
> > > > 00:11写道：
> > > > > > >
> > > > > > > >
> > > > > > > > Hi community
> > > > > > > >
> > > > > > > > I am starting a DISCUSS for "PIP-259: Make the config
> > > > > > > > httpMaxRequestHeaderSize of the pulsar web server
> > configurable".
> > > > > > > >
> > > > > > > > ### Motivation
> > > > > > > >
> > > > > > > > We have two ways to manage pulsar's resources:
> > > > > > > > - By client API (Can manage some resources, such as `create
> > topic`,
> > > > > > > `create
> > > > > > > > subscriber`, and so on)
> > > > > > > > - By admin API (Can manage all the resources)
> > > > > > > >
> > > > > > > > The `client API` has no limit on the request length. And the
> > > > `admin API`
> > > > > > > > has a limit on the request length(such as HTTP request line and
> > > > HTTP
> > > > > > > > request headers), this restriction is done by the built-in web
> > > > container
> > > > > > > > Jetty.
> > > > > > > >
> > > > > > > > Almost resources can be created by two APIs, but can only be
> > > > modified and
> > > > > > > > deleted by `admin API`. This causes us to be unable to modify
> > or
> > > > delete
> > > > > > > > resources created by `client API` with too long a name because
> > it
> > > > exceeds
> > > > > > > > Jetty's default HTTP request URI length limit.
> > > > > > > >
> > > > > > > > ### Goal
> > > > > > > >
> > > > > > > > #### 1. For web servers
> > > > > > > > Provide a way to modify Jetty's `httpMaxRequestHeaderSize`
> > > > configuration
> > > > > > > > (involves two servers: the web server in pulsar and the web
> > server
> > > > in
> > > > > > > > pulsar-proxy)
> > > > > > > >
> > > > > > > > #### 2.For the internal client in pulsar-proxy
> > > > > > > > Provide a way to modify Jetty-client's
> > > > `httpClientRequestBufferSize`
> > > > > > > > configuration.
> > > > > > > >
> > > > > > > > Since the pulsar-proxy handles HTTP requests like this：
> > > > `pulsar-admin.sh`
> > > > > > > > -> `proxy web server` -> `(highlight) internal client in
> > proxy` ->
> > > > > > > `pulsar
> > > > > > > > web server`.
> > > > > > > >
> > > > > > > > When the internal client forwards a request, it forwards the
> > > > request
> > > > > > > header
> > > > > > > > and the request body, and all the data passes through a
> > buffer( we
> > > > call
> > > > > > > it
> > > > > > > > Buf ), like this:
> > > > > > > > - Receive a request
> > > > > > > > - Put the request line and request headers input to the Buf.
> > > > > > > > - <strong>(highlight)</strong>Flush the Buf ( If the data in
> > the
> > > > request
> > > > > > > > line and request header exceeds the length of the buf, an
> > error is
> > > > > > > reported
> > > > > > > > )
> > > > > > > > - Put the request body input to the Buf.
> > > > > > > > - Flush the Buf if it is full.
> > > > > > > >
> > > > > > > > So we need a config to set the `buff size` of the Buf:
> > > > > > > > `pulsar-proxy.conf.httpClientRequestBufferSize` -> `buf size
> > of the
> > > > > > > > internal client`.
> > > > > > > >
> > > > > > > > ### API Changes
> > > > > > > >
> > > > > > > > #### ServiceConfiguration.java
> > > > > > > > ```java
> > > > > > > > @FieldContext(
> > > > > > > > category = CATEGORY_HTTP,
> > > > > > > > doc = """
> > > > > > > > The maximum size in bytes of the request
> > header.
> > > > > > > > Larger headers will allow for more and/or
> > larger
> > > > cookies
> > > > > > > > plus larger form content encoded in a URL.
> > > > > > > > However, larger headers consume more memory and
> > > > can make
> > > > > > > a
> > > > > > > > server more vulnerable to denial of service
> > > > > > > > attacks.
> > > > > > > > """
> > > > > > > > )
> > > > > > > > private int httpMaxRequestHeaderSize = 8 * 1024;
> > > > > > > > ```
> > > > > > > >
> > > > > > > > #### ProxyConfiguration.java
> > > > > > > >
> > > > > > > > ```java
> > > > > > > > @FieldContext(
> > > > > > > > minValue = 1,
> > > > > > > > category = CATEGORY_HTTP,
> > > > > > > > doc = """
> > > > > > > > The maximum size in bytes of the request
> > header.
> > > > > > > > Larger headers will allow for more and/or
> > larger
> > > > cookies
> > > > > > > > plus larger form content encoded in a URL.
> > > > > > > > However, larger headers consume more memory and
> > > > can make
> > > > > > > a
> > > > > > > > server more vulnerable to denial of service
> > > > > > > > attacks.
> > > > > > > > """
> > > > > > > > )
> > > > > > > > private int httpMaxRequestHeaderSize = 8 * 1024;
> > > > > > > >
> > > > > > > > @FieldContext(
> > > > > > > > minValue = 1,
> > > > > > > > category = CATEGORY_HTTP,
> > > > > > > > doc = """
> > > > > > > > the size of the buffer used to write requests
> > to
> > > > Broker.
> > > > > > > > if "httpMaxRequestHeaderSize" is large than
> > > > > > > > "httpClientRequestBufferSize", will set
> > > > > > > > "httpClientRequestBufferSize" to the value of
> > > > > > > > "httpMaxRequestHeaderSize"
> > > > > > > > """
> > > > > > > > )
> > > > > > > > private int httpClientRequestBufferSize =
> > > > httpMaxRequestHeaderSize;
> > > > > > > > ```
> > > > > > > >
> > > > > > > > ### Anything else?
> > > > > > > >
> > > > > > > > This change should cherry-pick into the previous branches (
> > > > includes
> > > > > > > > `2.9~2.11` )
> > > > > > > >
> > > > > > > > If the user uses the features `RETRY Topic` or `DLQ`, it is
> > > > possible that
> > > > > > > > pulsar will automatically create some topics with names that
> > are
> > > > too long
> > > > > > > > and cannot be managed, the [scenario has been discussed in the
> > > > email](
> > > > > > > >
> > https://lists.apache.org/thread/q1m23ckyy10wvtzy65v8bwqwnh7r0gc8)
> > > > before
> > > > > > > >
> > > > > > > >
> > > > > > > > Thanks
> > > > > > > > Yubiao Feng
> > > > > > >
> > > >
> >
