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

mon/MonClient: respect the priority in SRV RR #15964

Merged
merged 6 commits into from Jul 5, 2017

Conversation

Projects
None yet
3 participants
@tchaikov
Contributor

tchaikov commented Jun 28, 2017

tchaikov added some commits Jun 27, 2017

include/scope_guard.h: include <utility>
so this header is self-contained.

Signed-off-by: Kefu Chai <kchai@redhat.com>
common/dns_resolve: refactor DNSResolver::resolve_cname() to remove g…
…otos

trade gotos for better readablity

Signed-off-by: Kefu Chai <kchai@redhat.com>

@tchaikov tchaikov requested review from gregsfortytwo, jdurgin and rjfd Jun 28, 2017

@rjfd

rjfd approved these changes Jun 28, 2017

lgtm

@tchaikov tchaikov added the needs-qa label Jun 28, 2017

@tchaikov tchaikov requested a review from jecluis Jun 28, 2017

@gregsfortytwo

I don't think I like this much. It's transparently passing through the SRV data, but we don't have any way for admins who aren't doing the custom DNS route to configure connections correctly. And why is there both a priority and a weight? I don't mind them exactly but I do see it being confusing for admins -- we could probably just have priorities and randomly select within them and have an interface which is just as useful in practical terms. And I think we should design for our needs and then map the dns options into that instead of designing based on the dns options.
The internal structure stuff all looks fine, of course. :)

tchaikov added some commits Jun 28, 2017

mon: MonMap: add priority to mon_into_t
Signed-off-by: Kefu Chai <kchai@redhat.com>
mon/MonClient: respect the priority in mon_info_t
* common/dns_resolve: collect the priority in the SRV records
  also
* mon/MonClient: only connect to the mon with highest priority
  (in context of SRV record, the lowest priority value), and prefer
  the mon with higher weight.

Fixes: http://tracker.ceph.com/issues/5249
Signed-off-by: Kefu Chai <kchai@redhat.com>
common/dns_resolve: do not assert with misconfigured SRV RR
Signed-off-by: Kefu Chai <kchai@redhat.com>
doc: update with "mon priority" related changes
* doc/rados/configuration/mon-lookup-dns.rst
  we now partially support RFC2782: only the targets with the lowest
  value are selected
* doc/rados/configuration/network-config-ref.rst
  update with "mon priority" option in "[mon.$id]" section

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 29, 2017

changelog

  • read "mon priority" in [mon.*] section when building initial monmap, so administrators not using DNS SRV can also route the mon clients to mon with higher priority
  • drop the SRV "weight" support

@tchaikov tchaikov changed the title from mon/MonClient: respect the priority/weight in SRV RR to mon/MonClient: respect the priority in SRV RR Jun 29, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 29, 2017

@gregsfortytwo fixed and repushed.

@gregsfortytwo

This comment has been minimized.

Member

gregsfortytwo commented Jun 29, 2017

I much prefer this version, thanks!

@tchaikov tchaikov merged commit 4fd72b4 into ceph:master Jul 5, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
default Build finished.
Details
make check make check succeeded
Details

@tchaikov tchaikov deleted the tchaikov:wip-5249 branch Jul 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment