[GitHub] [pulsar] BewareMyPower edited a comment on pull request #14320: [Broker] Increase default numHttpServerThreads value to 200 to prevent Admin API unavailability

[GitBox <gi...@apache.org>]

BewareMyPower edited a comment on pull request #14320:
URL: https://github.com/apache/pulsar/pull/14320#issuecomment-1042545245


   > > We shouldn't add some temporary PRs during the release phase unless they are really important.
   > 
   > @BewareMyPower This PR contains a very important change. The recommended maximum thread count for Jetty thread pool is 50-500. Pulsar current uses an invalid valid. The sync->async changes in the Pulsar Admin API are not needed when a proper value is used.
   
   I think I need to explain more for the **important** term I used. IMO, a PR that could block a release during the release phase must match following rules:
   1. It must be a bug fix.
   2. The bug was introduced from the current release, i.e. it's a regression.
   3. There is no workaround.
   
   It's only my opinion. I think our release document for release manager missed something like this.
   
   Let's look back to this PR. First, I don't think a change to the default configuration value can be treated as a bug fix. It's more like an enhancement. Because the previous stable releases all should have the same problem. Then, we can see it's not a regression. Third, it's not something serious like Log4j2 Vulnerability (CVE-2021-44228). It just make some certain cases not work for Admin API and can be fixed by configuration tuning.
   
   In short, IMO, after a release started, we must be very careful and strict on the new PRs.


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org