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

Redesign for v1 #48

Merged
merged 42 commits into from
Oct 9, 2020
Merged

Redesign for v1 #48

merged 42 commits into from
Oct 9, 2020

Conversation

nicholaschiasson
Copy link
Owner

@nicholaschiasson nicholaschiasson commented Sep 20, 2020

Resolves #32
Resolves wdaike/ngx_upstream_jdomain#7
Resolves wdaike/ngx_upstream_jdomain#15

This change focuses on making the module leverage nginx's upstream server structure so it can be more compatible with nginx modules.

Each jdomain directive creates an underlying server object and also keeps track of the round robin peers which are created from that server so that their states can be manipulated directly.

This change further simplifies the module by relying on the default round robin load balancer built into nginx core. Instead of doing all of the peer initialization, manual peer selection and SSL session handling, this module calls the round robin initialization and round robin peer initialization, and all of those things are taken care of by nginx code. So basically this module only sets up handlers for the initialization and peer initialization phases of a load balancer module, whereas it used to handle init, init peer, get peer, free peer, set session, and save session... most of which is quite unnecessary. Now this module can focus on its sole responsibility, which is to resolve a domain on an interval and overwrite peer addresses if necessary.

Furthermore, this change aims to add much more thorough testing, especially where compatibility with other nginx modules is concerned.

TODO:

Primarily to leverage upstream server.
Make use of nginx default round robin load balancing.
Test using SSL also.
@nicholaschiasson nicholaschiasson added the enhancement New feature or request label Sep 20, 2020
@nicholaschiasson nicholaschiasson self-assigned this Sep 20, 2020
Discovered that the round robin load balancing allocates all of its peers upon initialization based on the servers in an upstream, and then never again. I decided to not mess with this mechanism and instead just let it initialize then find and cache pointers to the round robin peers that point to my jdomain addresses. That allows me to set the peers down state, since changing that on the server has no effect after the round robin initialization has run.
I also decided to allocate the max IPs worth of peers up front and set the server's naddrs to the max IPs in order to trick the round robin initialization into also allocating that many peers from the start. I then reset the server naddrs to the number of IPs we actually resolved, and set the offset of peers to down so they aren't used. It feels a little bit like a hack, but it is the safest thing to do as far as memory management goes.
@nicholaschiasson nicholaschiasson changed the title Rewrite module Redesign for v1 Sep 23, 2020
@nicholaschiasson nicholaschiasson marked this pull request as ready for review September 28, 2020 12:44
@nicholaschiasson
Copy link
Owner Author

Compatibility with virtual host traffic status module is as good as it can be but that module has a shortcoming in which it only considers one address in a server. I have raised this issue to start the conversation, but we will just have to live with it for now. I may open a PR for that myself.

@nicholaschiasson nicholaschiasson merged commit 057c08f into master Oct 9, 2020
@nicholaschiasson nicholaschiasson deleted the major/use-nginx-server branch October 9, 2020 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

是否支持hash key health check wdaike/ngx_upstream_jdomain#7
1 participant