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

Periodic DNS resolution #136

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

onlynone
Copy link
Contributor

This should allow both the oss and plus versions to do periodic DNS resolution by using a variable in the proxy_pass and eliminating the jump through an upstream.

@dekobon dekobon self-requested a review May 31, 2023 23:16
@dekobon
Copy link
Collaborator

dekobon commented Jun 1, 2023

Thank you for taking the time to put together this PR.

Unfortunately, there are some invisible side-effects of this change:

  • Loss of keep-alive connections
  • Loss of statistics for upstream

Also, DNS resolution for OSS is currently in review, so first-class support for it should be coming in the near term.

Out of the above issue, the loss of keep-alive connections is a significant drawback for the S3 use case. Please let me know what your thoughts are.

@dekobon
Copy link
Collaborator

dekobon commented Jun 1, 2023

By the way, I cherry-picked in the commit for fixing the $s3uri case.

@onlynone
Copy link
Contributor Author

onlynone commented Jun 1, 2023

Also, DNS resolution for OSS is currently in review, so first-class support for it should be coming in the near term.

That would be great. I hope that feature gets merged soon.

Out of the above issue, the loss of keep-alive connections is a significant drawback for the S3 use case. Please let me know what your thoughts are.

Can you explain how significant the keep-alive connections are with S3? Is there no way to get keep-alive connections without going through an upstream?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants