Please find the response inline. On Wed, Jun 21, 2023 at 5:53 PM PengHui Li <peng...@apache.org> wrote:
> > However, stats retrieval over HTTP API doesn’t work well in use cases > when users would like to access this API at a higher scale when a large > number of application nodes would like to use it over HTTP which could > overload brokers and sometimes makes broker irresponsive and impact admin > API performance. It’s also become difficult when Pulsar is deployed in the > cloud behind the SNI proxy and applications also want to access large-scale > stats information periodically over different HTTP port instead it would be > better if applications can fetch stats over on same binary protocol for > scalability and accessibility reasons. > > From the motivation of the proposal, resolving the performance issue with > the REST API > is the goal. Sorry, I haven't run a benchmark for the REST API, have you > tested it or could > you please figure out where is the bottleneck with the REST API solution? > And if users use > the new approach, what is the performance expectation here, get a 30% or > 50% improvement? > >> Main reasons are performance (you will see improvement due to http overhead compare to tcp, limited threads on http-server side and definitely performance compare to netty-tcp vs http-jetty server), user feasibility (same reason why we have producer/consumer stats using binary protocol where user can get the stats from the same producer/consumer entity without having additional client creation for the stats), scale (allowing large number of requests over binary protocol compare to http is more scalable). > And I'm not sure why it is difficult to fetch the stats behind the SNI > proxy, could you please explain > more? It will be helpful for the reviewers to understand the issue we want > to resolve. > >> SNI is layer-4 tcp layer protocol and does not support over the http. > For the client side API changes. You have InternalStatsOption as a param of > the method, but > I don't see any definition for it. It should be enum? And why not have a > pojo for it? It will be > more easier for users to know what should be set and what should not. > I have added that param map after our last discussion which will be enum to pass additional flags. > For the response data. The JSON string is not good for compatibility. And I > saw the discussion > in the DISCUSSION thread from you and Enrico. But it's not a guaranteed > solution from the API's > perspective. And for any other clients, cpp, go, rust, they must build > their own pojo based on > the REST API. > Well JSON would be the better option to avoid data conversion and copy for complex stats/internal-stats complex data-structures at broker and client side, maintaining consistency between stats/internal-stats pojo-schema, and accessing stats with consistent pojo definition. > Thanks, > Penghui > > On Tue, Jun 20, 2023 at 3:06 PM Rajan Dhabalia <rdhaba...@apache.org> > wrote: > > > Hi, > > > > I would like to start VOTE for : > > https://github.com/apache/pulsar/issues/20265 > > > > Thanks, > > Rajan > > >
