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

gateway: DNS SRV priority #7882

Merged
merged 2 commits into from
May 8, 2017
Merged

Conversation

heyitsanthony
Copy link
Contributor

Fixes #4378

Trying to decouple the v2 client from SRV code. Can't move
into discovery/ since that creates a circular dependency. So,
give up and move all the SRV code into a new package.
w += int(r.srv.Weight) + 1
}
}
// randomly choose by weight
Copy link
Contributor

Choose a reason for hiding this comment

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

A server selection mechanism. The weight field specifies a
relative weight for entries with the same priority. Larger
weights SHOULD be given a proportionately higher probability of
being selected. The range of this number is 0-65535. This is a
16 bit unsigned integer in network byte order. Domain
administrators SHOULD use Weight 0 when there isn't any server
selection to do, to make the RR easier to read for humans (less
noisy). In the presence of records containing weights greater
than 0, records with weight 0 should have a very small chance of
being selected.

Can we follow this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

@xiang90
Copy link
Contributor

xiang90 commented May 5, 2017

@heyitsanthony

Is there a plan for normal endpoints (like http://127.0.0.1:2379[priority:10,weight:80]) ?

@heyitsanthony
Copy link
Contributor Author

@xiang90 no. That format was never discussed. Do any other systems do it that way?

@xiang90
Copy link
Contributor

xiang90 commented May 5, 2017

@heyitsanthony

Do any other systems do it that way?

Not I know of. I am not proposing the format here. But, it is important that we can support weighed routing in SRV discovery and other mechanisms similarly.

@heyitsanthony
Copy link
Contributor Author

This is just for the gateway. Anything using the client would need a new balancer to do that; it is way too late into the 3.2.0 cycle to have that without releasing a broken balancer.

@@ -67,6 +68,7 @@ func newGatewayStartCommand() *cobra.Command {

cmd.Flags().StringVar(&gatewayListenAddr, "listen-addr", "127.0.0.1:23790", "listen address")
cmd.Flags().StringVar(&gatewayDNSCluster, "discovery-srv", "", "DNS domain used to bootstrap initial cluster")
cmd.Flags().StringVar(&gatewayEndpointPolicy, "endpoint-policy", "round-robin", "Policy for selecting next connection's endpoint (round-robin, srv-priority)")
Copy link
Contributor

Choose a reason for hiding this comment

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

if they choose to use dns srv, the priority policy should always be respected, right? i do not think we need this flag.

@heyitsanthony heyitsanthony force-pushed the srv-priority branch 3 times, most recently from b47c5d4 to 10b092e Compare May 8, 2017 18:32
Adds DNS SRV weighting and priorities to gateway.

Partially addresses etcd-io#4378
@xiang90
Copy link
Contributor

xiang90 commented May 8, 2017

lgtm

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@2655540). Click here to learn what that means.
The diff coverage is 41.4%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #7882   +/-   ##
=========================================
  Coverage          ?   75.78%           
=========================================
  Files             ?      332           
  Lines             ?    26278           
  Branches          ?        0           
=========================================
  Hits              ?    19914           
  Misses            ?     4934           
  Partials          ?     1430
Impacted Files Coverage Δ
etcdmain/grpc_proxy.go 20.63% <0%> (ø)
etcdmain/util.go 0% <0%> (ø)
embed/config.go 69.23% <0%> (ø)
client/discover.go 0% <0%> (ø)
etcdmain/gateway.go 38.46% <0%> (ø)
proxy/tcpproxy/userspace.go 58.77% <60.86%> (ø)
pkg/srv/srv.go 70% <69.44%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2655540...c232814. Read the comment docs.

@heyitsanthony heyitsanthony merged commit aac2292 into etcd-io:master May 8, 2017
@heyitsanthony heyitsanthony deleted the srv-priority branch May 8, 2017 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants