Skip to content

Periodic DNS resolution#136

Closed
onlynone wants to merge 3 commits intonginx:masterfrom
onlynone:dns_resolution
Closed

Periodic DNS resolution#136
onlynone wants to merge 3 commits intonginx:masterfrom
onlynone:dns_resolution

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?

@dekobon
Copy link
Collaborator

dekobon commented May 18, 2025

DNS resolution has been open sourced: https://blog.nginx.org/blog/dynamic-dns-resolution-open-sourced-in-nginx

Closing in favor of changes that use the OSS approach

@dekobon dekobon closed this May 18, 2025
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