Skip to content

[Broker][Proxy][Function worker] Remove duplicate servlet filter instances to fix backpressure#15637

Merged
lhotari merged 1 commit into
apache:masterfrom
lhotari:lh-remove-duplicate-web-filters
Jun 6, 2022
Merged

[Broker][Proxy][Function worker] Remove duplicate servlet filter instances to fix backpressure#15637
lhotari merged 1 commit into
apache:masterfrom
lhotari:lh-remove-duplicate-web-filters

Conversation

@lhotari

@lhotari lhotari commented May 17, 2022

Copy link
Copy Markdown
Member

Motivation

Follow up on #14353

There were filter instances for each context path which made maxConcurrentHttpRequests and httpRequestsMaxPerSecond configuration options not work as expected. The actual limits would be multiple times higher dependencing on the way clients access different context paths.

Modifications

  • Refactor solution to use FilterHolder for sharing Servlet Filter instances across multiple servlet context paths
  • Fix invalid Jersey API usage
    • config.packages("jersey.config.server.provider.packages", javaPackages); is invalid
    • The API takes a java package directly, passing jersey.config.server.provider.packages doesn't make any sense.
    • When configuring individual resource classes, .register method should be used instead of registering the package which includes the individual resource class.
    • Don't use recursive resource class scanning since it might results in unintended resource classes being exposed.

…r web server

- There were filter instances for each context path which made
  maxConcurrentHttpRequests and httpRequestsMaxPerSecond not work as expected.
- Fixes the backpressure solution that is dependent on maxConcurrentHttpRequests and httpRequestsMaxPerSecond
  working properly
- Fix invalid Jersey api usage

@michaeljmarshall michaeljmarshall left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lhotari lhotari closed this Jun 3, 2022
@lhotari lhotari reopened this Jun 3, 2022

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Nice work.
I like the fact that we won't be doing classpath scanning anymore

@lhotari
lhotari merged commit 94392a4 into apache:master Jun 6, 2022
nodece pushed a commit to nodece/pulsar that referenced this pull request Jul 24, 2024
…r web server (apache#15637)

- There were filter instances for each context path which made
  maxConcurrentHttpRequests and httpRequestsMaxPerSecond not work as expected.
- Fixes the backpressure solution that is dependent on maxConcurrentHttpRequests and httpRequestsMaxPerSecond
  working properly
- Fix invalid Jersey api usage

(cherry picked from commit 94392a4)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants