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


   > > @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.
   > 
   > @lhotari This is not a new thing and it's something that can easily be 
changed by users. I don't think we need to rush into a release.
   
   I agree. Have I been rushing this? Instead, you can say that a lot of PRs 
with[ "make 
async"](https://github.com/apache/pulsar/pulls?q=is%3Apr+make+async) have been 
pushed and merged recently.
   This has introduced several known regressions that have been fixed. We don't 
know which are regressions that just haven't been found yet.
   
   [My valid questions were never answered by the contributors of the "make 
async" 
changes](https://github.com/apache/pulsar/issues/14013#issuecomment-1033528348).
   
   
   
![image](https://user-images.githubusercontent.com/66864/154451547-0e9937fb-ba50-425e-b9f0-5e77cdbd653c.png)
   
   I'm expecting that there are issues or a PIP which is referred to. 
   @merlimat WDYT?
   
   > 
   > > The sync->async changes in the Pulsar Admin API are not needed when a 
proper value is used.
   > 
   > The problem is not just keeping threads busy but the cases in which you 
have calls that spawn to multiple brokers. In this cases, the HTTP call can 
come back to same broker and we can have a deadlock, no matters how many 
threads we have in the pool.
   
   I'll clarify: what I have been referring to as "sync -> async" changes: 
changes where the use of the blocking Servlet API is migrated to use 
Asynchronous Servlet API. That won't solve any problems on it's own. Any 
problems that it might solve would be solved also by configuring Jetty as it is 
recommended to be configured when there are blocking calls involved. 
   The recommended maximum thread pool size is 50 to 500 for Jetty. I have been 
going through some details and there are multiple other things that are not 
properly configured. I'll be following up with separate PRs. (UPDATE: the draft 
PR which fixes backpressure handling is #14353).
   
   I'd assume that the reason for deadlocks when thread pool size is properly 
configured are caused by locks. I like to see an example of a deadlock which 
couldn't be resolved by continuing to use the blocking servlet api. I'm not 
against the changes from blocking API to async API, but I think changes need 
proper justification, especially when the "make async" changes have been 
initiated without referring any reported issues or a PIP.
   
   
   
   
   
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to