+1
Regards Jiwei Guo (Tboy) On Tue, Mar 21, 2023 at 4:21 PM Zike Yang <z...@apache.org> wrote: > > > To keep the goal clear, we can only set `httpClientRequestBufferSize` > internally without giving the user a way to change it. > > +1. > > Thanks, > Zike Yang > > On Tue, Mar 21, 2023 at 4:08 PM Yubiao Feng > <yubiao.f...@streamnative.io.invalid> wrote: > > > > Hi Zike > > > > > 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. > > > > To keep the goal clear, we can only set `httpClientRequestBufferSize` > > internally without giving the user a way to change it. > > > > > > 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 > > > > > > > > > > > > > > > >
