Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue-11966][pulsar-proxy] set default http proxy request timeout #11971

Merged
merged 2 commits into from
Oct 6, 2021

Conversation

zzzming
Copy link
Contributor

@zzzming zzzming commented Sep 8, 2021

Fixes #11966

Address missing http proxy request timeout in issue #11966

Motivation

Set http proxy request timeout in pulsar-proxy

Modifications

Set default http proxy request timeout in pulsar-proxy as 30 seconds

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: introduced a new default of proxy HTTP connect timeout
  • The wire protocol: no
  • The rest endpoints: no
  • The admin cli options: no
  • Anything that affects deployment: no

@Anonymitaet
Copy link
Member

@zzzming Thanks for your contribution. For this PR, do we need to update docs?

(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@lhotari lhotari added area/proxy type/bug The PR fixed a bug or issue reported a bug labels Sep 9, 2021
@lhotari lhotari added this to the 2.9.0 milestone Sep 9, 2021
@lhotari lhotari changed the title [Issue-11966][pulsar-proxy]set default proxy http client connect timeout [Issue-11966][pulsar-proxy] set default proxy request timeout Sep 9, 2021
@lhotari lhotari changed the title [Issue-11966][pulsar-proxy] set default proxy request timeout [Issue-11966][pulsar-proxy] set default http proxy request timeout Sep 9, 2021
@eolivelli
Copy link
Contributor

@codelipenghui @rdhabalia @merlimat
it would be good to see this in 2.9.0 as it is a critical issue

@eolivelli eolivelli added the release/blocker Indicate the PR or issue that should block the release until it gets resolved label Sep 20, 2021
@eolivelli eolivelli modified the milestones: 2.9.0, 2.10.0 Oct 6, 2021
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Lgtm

@eolivelli eolivelli merged commit 8b55636 into apache:master Oct 6, 2021
@merlimat
Copy link
Contributor

merlimat commented Oct 6, 2021

@zzzming @lhotari @eolivelli this change does not take into consideration that upload and download of very large functions jars can easily take more than 30 sec.

@lhotari
Copy link
Member

lhotari commented Oct 6, 2021

@zzzming @lhotari @eolivelli this change does not take into consideration that upload and download of very large functions jars can easily take more than 30 sec.

@merlimat ok, what would be a sufficient timeout value?

@merlimat
Copy link
Contributor

merlimat commented Oct 6, 2021

Maybe we could try with something like 5min?

@eolivelli
Copy link
Contributor

@merlimat you are right.
We should change the default

@zzzming would you mind sending a new PR?

@eolivelli eolivelli modified the milestones: 2.10.0, 2.9.0 Oct 8, 2021
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Oct 8, 2021
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Oct 8, 2021
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Oct 11, 2021
codelipenghui pushed a commit that referenced this pull request Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy cherry-picked/branch-2.8 Archived: 2.8 is end of life release/blocker Indicate the PR or issue that should block the release until it gets resolved release/2.8.2 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proxy] Proxied /admin endpoint connections might remain blocked forever
8 participants