[GitHub] [pulsar] lhotari commented on a diff in pull request #15637: [Broker][Proxy][Function worker] Remove duplicate servlet filter instances to fix backpressure

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

lhotari commented on code in PR #15637:
URL: https://github.com/apache/pulsar/pull/15637#discussion_r888820094


##########
pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/WorkerServer.java:
##########
@@ -171,18 +174,52 @@ private void init() {
         server.setConnectors(connectors.toArray(new ServerConnector[connectors.size()]));
     }
 
-    public static ServletContextHandler newServletContextHandler(String contextPath,
+    private static class FilterInitializer {

Review Comment:
   > I see that this class is duplicated across several classes. What is the motivation for duplicating the code?
   
   I don't see a big difference whether the logic is in this new class compared to the previous duplication which is in methods. This PR didn't introduce the duplication which was in methods before moving it to these internal FilterInitializer classes. 
   
   I agree that it would be useful to refactor the solution to get rid of the duplication. I was thinking of that and also did a short spike, but the end result would have been a large refactoring. This would make it harder to understand what was changed in this particular case for getting rid of the duplicate servlet filter instances.



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