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 <[email protected]> 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
> <[email protected]> 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 <[email protected]> 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 丛搏 <[email protected]> wrote:
> > > >
> > > > +1
> > > >
> > > > Hi, Yubiao :
> > > >
> > > > Thanks for your explanation. That makes sense to me.
> > > >
> > > > Thanks,
> > > > Bo
> > > >
> > > >
> > > > Yubiao Feng <[email protected]> 于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 丛搏 <[email protected]> wrote:
> > > > >
> > > > > > hi yubiao :
> > > > > >
> > > > > > I have a question, why we need `httpClientRequestBufferSize ` in
> > > > > > proxy, can you explain in detail?
> > > > > >
> > > > > > Thanks,
> > > > > > Bo
> > > > > >
> > > > > > Yubiao Feng <[email protected]> 于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
> > > > > >
> > >
>

Reply via email to