Re: [ANNOUNCE] autofs 5.1.2 release
On 21/12/17 09:09, NeilBrown wrote: > 8<--- > Subject: use_hostname_for_mounts shouldn't prevent selection among replica > > If several replicas have been specified for a mount point, and > use_hostname_for_mount is set to "yes", the selection between > these replicas is currently disabled and the last in the list is always > chosen. > > There is little point selecting between different interfaces on the one > host in this case, but it is still worth selecting between different > hosts, particularly if different weights have been specified. > > This patch restores the "prune_host_list()" functionality when > use_hostname_for_mount is set, and modifies it slightly so that once > an IP address with a given proximity has been successfully probed, > other IP address for the same host(weight):/path and proximity are ignored. > > Signed-off-by: NeilBrown> > diff --git a/modules/replicated.c b/modules/replicated.c > index 3ac4c70f4062..16cf873513ff 100644 > --- a/modules/replicated.c > +++ b/modules/replicated.c > @@ -714,7 +714,7 @@ done: > int prune_host_list(unsigned logopt, struct host **list, > unsigned int vers, int port) > { > - struct host *this, *last, *first; > + struct host *this, *last, *first, *prev; > struct host *new = NULL; > unsigned int proximity, selected_version = 0; > unsigned int v2_tcp_count, v3_tcp_count, v4_tcp_count; > @@ -726,12 +726,6 @@ int prune_host_list(unsigned logopt, struct host **list, > if (!*list) > return 0; > > - /* If we're using the host name then there's no point probing > - * avialability and respose time. > - */ > - if (defaults_use_hostname_for_mounts()) > - return 1; > - > /* Use closest hosts to choose NFS version */ > > first = *list; > @@ -877,11 +871,18 @@ int prune_host_list(unsigned logopt, struct host **list, > > first = last; > this = first; > + prev = NULL; > while (this) { > struct host *next = this->next; > if (!this->name) { > remove_host(list, this); > add_host(, this); > + } else if (defaults_use_hostname_for_mounts() && prev && > +prev->proximity == this->proximity && > +strcmp(prev->name, this->name) == 0 && > +strcmp(prev->path, this->path) == 0 && > +prev->weight == this->weight) { > + /* No need to probe same host(weight):/path again */ Mmm ... so maybe I'm the one that's missing the point. You are trying to eliminate multiple occurrences of list entries that correspond to a specific host name entry from probing. It might be sensible to add a "this->rr" following the defaults_use_hostname_for_mounts() check to avoid the additional checks when the host doesn't have additional addresses, particularly the string comparison. There's nothing stopping people from adding this same host name with a different weight, even though that doesn't seem like a sensible thing to do. I'm not sure if this exposes mounting to problems that aren't already present with the current implementation. I'll think a little more about that case but at first glance the DNS round robin problem of addresses referring to different devices is still present, a possible false negative. But that problem exits in the current implementation too as a round robin lookup can just as easily return an address of a host that isn't responding at mount time. > } else { > status = get_supported_ver_and_cost(logopt, this, > selected_version, port); > @@ -889,6 +890,7 @@ int prune_host_list(unsigned logopt, struct host **list, > this->version = selected_version; > remove_host(list, this); > add_host(, this); > + prev = this; > } > } > this = next; >
Re: [ANNOUNCE] autofs 5.1.2 release
On 21/12/17 09:09, NeilBrown wrote: > 8<--- > Subject: use_hostname_for_mounts shouldn't prevent selection among replica > > If several replicas have been specified for a mount point, and > use_hostname_for_mount is set to "yes", the selection between > these replicas is currently disabled and the last in the list is always > chosen. > > There is little point selecting between different interfaces on the one > host in this case, but it is still worth selecting between different > hosts, particularly if different weights have been specified. > > This patch restores the "prune_host_list()" functionality when > use_hostname_for_mount is set, and modifies it slightly so that once > an IP address with a given proximity has been successfully probed, > other IP address for the same host(weight):/path and proximity are ignored. > > Signed-off-by: NeilBrown > > diff --git a/modules/replicated.c b/modules/replicated.c > index 3ac4c70f4062..16cf873513ff 100644 > --- a/modules/replicated.c > +++ b/modules/replicated.c > @@ -714,7 +714,7 @@ done: > int prune_host_list(unsigned logopt, struct host **list, > unsigned int vers, int port) > { > - struct host *this, *last, *first; > + struct host *this, *last, *first, *prev; > struct host *new = NULL; > unsigned int proximity, selected_version = 0; > unsigned int v2_tcp_count, v3_tcp_count, v4_tcp_count; > @@ -726,12 +726,6 @@ int prune_host_list(unsigned logopt, struct host **list, > if (!*list) > return 0; > > - /* If we're using the host name then there's no point probing > - * avialability and respose time. > - */ > - if (defaults_use_hostname_for_mounts()) > - return 1; > - > /* Use closest hosts to choose NFS version */ > > first = *list; > @@ -877,11 +871,18 @@ int prune_host_list(unsigned logopt, struct host **list, > > first = last; > this = first; > + prev = NULL; > while (this) { > struct host *next = this->next; > if (!this->name) { > remove_host(list, this); > add_host(, this); > + } else if (defaults_use_hostname_for_mounts() && prev && > +prev->proximity == this->proximity && > +strcmp(prev->name, this->name) == 0 && > +strcmp(prev->path, this->path) == 0 && > +prev->weight == this->weight) { > + /* No need to probe same host(weight):/path again */ Mmm ... so maybe I'm the one that's missing the point. You are trying to eliminate multiple occurrences of list entries that correspond to a specific host name entry from probing. It might be sensible to add a "this->rr" following the defaults_use_hostname_for_mounts() check to avoid the additional checks when the host doesn't have additional addresses, particularly the string comparison. There's nothing stopping people from adding this same host name with a different weight, even though that doesn't seem like a sensible thing to do. I'm not sure if this exposes mounting to problems that aren't already present with the current implementation. I'll think a little more about that case but at first glance the DNS round robin problem of addresses referring to different devices is still present, a possible false negative. But that problem exits in the current implementation too as a round robin lookup can just as easily return an address of a host that isn't responding at mount time. > } else { > status = get_supported_ver_and_cost(logopt, this, > selected_version, port); > @@ -889,6 +890,7 @@ int prune_host_list(unsigned logopt, struct host **list, > this->version = selected_version; > remove_host(list, this); > add_host(, this); > + prev = this; > } > } > this = next; >
Re: [ANNOUNCE] autofs 5.1.2 release
On 03/01/18 06:14, NeilBrown wrote: > On Thu, Dec 21 2017, Ian Kent wrote: > >> On 21/12/17 19:06, Ian Kent wrote: >>> On 21/12/17 09:09, NeilBrown wrote: On Wed, Dec 20 2017, Ian Kent wrote: > On 20/12/17 13:52, Ian Kent wrote: >> On 20/12/17 11:29, NeilBrown wrote: >>> >>> Hi Ian, >>> I've been looking at: >>> - add configuration option to use fqdn in mounts. >>> >>> (commit 9aeef772604) because using this new option causes a regression. >>> If you are using the "replicated server" functionality, then >>> use_hostname_for_mounts = yes >>> completely disables it. >> >> Yes, that's not quite right. >> >> It disables the probe and proximity check for each distinct host >> name used. >> >> Each of the entries in the list of hosts should still be >> attempted and given that NFS ping is also now used in the NFS >> mount module what's lost is the preferred ordering of the hosts >> list. >> >>> >>> This is caused by: >>> >>> diff --git a/modules/replicated.c b/modules/replicated.c >>> index 32860d5fe245..8437f5f3d5b2 100644 >>> --- a/modules/replicated.c >>> +++ b/modules/replicated.c >>> @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host >>> **list, >>> if (!*list) >>> return 0; >>> >>> + /* If we're using the host name then there's no point probing >>> +* avialability and respose time. >>> +*/ >>> + if (defaults_use_hostname_for_mounts()) >>> + return 1; >>> + >>> /* Use closest hosts to choose NFS version */ >>> >>> My question is: why what this particular change made. >> >> It was a while ago but there were complains about using the IP >> address for mounts. It was requested to provide a way to prevent >> that and force the use of the host name in mounts. >> >>> Why can't prune_host_list() be allowed to do it's thing >>> when use_hostname_for_mounts is set. >> >> We could if each host name resolved to a single IP address. >> >> I'd need to check that use_hostname_for_mounts doesn't get >> in the road but the host struct should have ->rr set to true >> if it has multiple addresses so changing it to work the way >> your recommending shouldn't be hard. I think there's a couple >> of places that would need to be checked. >> >> If the host does resolve to multiple addresses the situation >> is different. There's no way to stop the actual mount from >> trying an IP address that's not responding and proximity >> doesn't make sense either again because every time a lookup >> is done on the host name (eg. at mount time) the next address >> in its list will be returned which can and usually is different >> from what would have been checked. >> >>> I understand that it would be pointless choosing between >>> the different interfaces of a multi-homed host, but there is still value >>> in choosing between multiple distinct hosts. >>> >>> What, if anything, might go wrong if I simply reverse this chunk of the >>> patch? >> >> You'll get IP addresses in the logs in certain cases but that >> should be all. >> >> It would probably be better to ensure that the checks are done >> if the host name resolves to a single IP address. > > I think that should be "if the host names in the list each resolve > to a single IP address", otherwise the round robin behavior would > probably still get in the road. I cannot see why the round-robin behavior would get in the road. It might be pointless to probe each IP address on a multi-homed host if we are just going to mount by host name, but I don't see how it hurts. >>> >>> I was wondering whether I had considered the configuration option >>> when setting ->rr of the host struct. I haven't so it should be set >>> if the name resolves to multiple addresses. >>> So this is what I'm thinking. Some simple testing suggests that it does the right things. If a host has addresses with different proximity they will still be probed separately, but this won't affect the final choice. >>> >>> Well no but ... >>> >>> The prune_host_list() function is meant to put the list of hosts >>> in proximity order with hosts ordered by response time within proximity. >>> >>> It's also meant to remove hosts that don't respond from the list so that >>> mount attempts are not made to them (hence the need to use address). >>> >>> When use_hostname_for_mounts is set there will be multiple host structures >>> with the same host name and each would be probed by name so all would be >>> considered. >>> >>> But the probe is not tied to IP address so each probe failure would remove >>> one of the structures from the list resulting
Re: [ANNOUNCE] autofs 5.1.2 release
On 03/01/18 06:14, NeilBrown wrote: > On Thu, Dec 21 2017, Ian Kent wrote: > >> On 21/12/17 19:06, Ian Kent wrote: >>> On 21/12/17 09:09, NeilBrown wrote: On Wed, Dec 20 2017, Ian Kent wrote: > On 20/12/17 13:52, Ian Kent wrote: >> On 20/12/17 11:29, NeilBrown wrote: >>> >>> Hi Ian, >>> I've been looking at: >>> - add configuration option to use fqdn in mounts. >>> >>> (commit 9aeef772604) because using this new option causes a regression. >>> If you are using the "replicated server" functionality, then >>> use_hostname_for_mounts = yes >>> completely disables it. >> >> Yes, that's not quite right. >> >> It disables the probe and proximity check for each distinct host >> name used. >> >> Each of the entries in the list of hosts should still be >> attempted and given that NFS ping is also now used in the NFS >> mount module what's lost is the preferred ordering of the hosts >> list. >> >>> >>> This is caused by: >>> >>> diff --git a/modules/replicated.c b/modules/replicated.c >>> index 32860d5fe245..8437f5f3d5b2 100644 >>> --- a/modules/replicated.c >>> +++ b/modules/replicated.c >>> @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host >>> **list, >>> if (!*list) >>> return 0; >>> >>> + /* If we're using the host name then there's no point probing >>> +* avialability and respose time. >>> +*/ >>> + if (defaults_use_hostname_for_mounts()) >>> + return 1; >>> + >>> /* Use closest hosts to choose NFS version */ >>> >>> My question is: why what this particular change made. >> >> It was a while ago but there were complains about using the IP >> address for mounts. It was requested to provide a way to prevent >> that and force the use of the host name in mounts. >> >>> Why can't prune_host_list() be allowed to do it's thing >>> when use_hostname_for_mounts is set. >> >> We could if each host name resolved to a single IP address. >> >> I'd need to check that use_hostname_for_mounts doesn't get >> in the road but the host struct should have ->rr set to true >> if it has multiple addresses so changing it to work the way >> your recommending shouldn't be hard. I think there's a couple >> of places that would need to be checked. >> >> If the host does resolve to multiple addresses the situation >> is different. There's no way to stop the actual mount from >> trying an IP address that's not responding and proximity >> doesn't make sense either again because every time a lookup >> is done on the host name (eg. at mount time) the next address >> in its list will be returned which can and usually is different >> from what would have been checked. >> >>> I understand that it would be pointless choosing between >>> the different interfaces of a multi-homed host, but there is still value >>> in choosing between multiple distinct hosts. >>> >>> What, if anything, might go wrong if I simply reverse this chunk of the >>> patch? >> >> You'll get IP addresses in the logs in certain cases but that >> should be all. >> >> It would probably be better to ensure that the checks are done >> if the host name resolves to a single IP address. > > I think that should be "if the host names in the list each resolve > to a single IP address", otherwise the round robin behavior would > probably still get in the road. I cannot see why the round-robin behavior would get in the road. It might be pointless to probe each IP address on a multi-homed host if we are just going to mount by host name, but I don't see how it hurts. >>> >>> I was wondering whether I had considered the configuration option >>> when setting ->rr of the host struct. I haven't so it should be set >>> if the name resolves to multiple addresses. >>> So this is what I'm thinking. Some simple testing suggests that it does the right things. If a host has addresses with different proximity they will still be probed separately, but this won't affect the final choice. >>> >>> Well no but ... >>> >>> The prune_host_list() function is meant to put the list of hosts >>> in proximity order with hosts ordered by response time within proximity. >>> >>> It's also meant to remove hosts that don't respond from the list so that >>> mount attempts are not made to them (hence the need to use address). >>> >>> When use_hostname_for_mounts is set there will be multiple host structures >>> with the same host name and each would be probed by name so all would be >>> considered. >>> >>> But the probe is not tied to IP address so each probe failure would remove >>> one of the structures from the list resulting
Re: [ANNOUNCE] autofs 5.1.2 release
On Thu, Dec 21 2017, Ian Kent wrote: > On 21/12/17 19:06, Ian Kent wrote: >> On 21/12/17 09:09, NeilBrown wrote: >>> On Wed, Dec 20 2017, Ian Kent wrote: >>> On 20/12/17 13:52, Ian Kent wrote: > On 20/12/17 11:29, NeilBrown wrote: >> >> Hi Ian, >> I've been looking at: >> >>> - add configuration option to use fqdn in mounts. >> >> (commit 9aeef772604) because using this new option causes a regression. >> If you are using the "replicated server" functionality, then >> use_hostname_for_mounts = yes >> completely disables it. > > Yes, that's not quite right. > > It disables the probe and proximity check for each distinct host > name used. > > Each of the entries in the list of hosts should still be > attempted and given that NFS ping is also now used in the NFS > mount module what's lost is the preferred ordering of the hosts > list. > >> >> This is caused by: >> >> diff --git a/modules/replicated.c b/modules/replicated.c >> index 32860d5fe245..8437f5f3d5b2 100644 >> --- a/modules/replicated.c >> +++ b/modules/replicated.c >> @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host >> **list, >> if (!*list) >> return 0; >> >> + /* If we're using the host name then there's no point probing >> +* avialability and respose time. >> +*/ >> + if (defaults_use_hostname_for_mounts()) >> + return 1; >> + >> /* Use closest hosts to choose NFS version */ >> >> My question is: why what this particular change made. > > It was a while ago but there were complains about using the IP > address for mounts. It was requested to provide a way to prevent > that and force the use of the host name in mounts. > >> Why can't prune_host_list() be allowed to do it's thing >> when use_hostname_for_mounts is set. > > We could if each host name resolved to a single IP address. > > I'd need to check that use_hostname_for_mounts doesn't get > in the road but the host struct should have ->rr set to true > if it has multiple addresses so changing it to work the way > your recommending shouldn't be hard. I think there's a couple > of places that would need to be checked. > > If the host does resolve to multiple addresses the situation > is different. There's no way to stop the actual mount from > trying an IP address that's not responding and proximity > doesn't make sense either again because every time a lookup > is done on the host name (eg. at mount time) the next address > in its list will be returned which can and usually is different > from what would have been checked. > >> I understand that it would be pointless choosing between >> the different interfaces of a multi-homed host, but there is still value >> in choosing between multiple distinct hosts. >> >> What, if anything, might go wrong if I simply reverse this chunk of the >> patch? > > You'll get IP addresses in the logs in certain cases but that > should be all. > > It would probably be better to ensure that the checks are done > if the host name resolves to a single IP address. I think that should be "if the host names in the list each resolve to a single IP address", otherwise the round robin behavior would probably still get in the road. >>> >>> I cannot see why the round-robin behavior would get in the road. >>> It might be pointless to probe each IP address on a multi-homed host if >>> we are just going to mount by host name, but I don't see how it hurts. >> >> I was wondering whether I had considered the configuration option >> when setting ->rr of the host struct. I haven't so it should be set >> if the name resolves to multiple addresses. >> >>> >>> So this is what I'm thinking. Some simple testing suggests that >>> it does the right things. >>> >>> If a host has addresses with different proximity they will still be >>> probed separately, but this won't affect the final choice. >> >> Well no but ... >> >> The prune_host_list() function is meant to put the list of hosts >> in proximity order with hosts ordered by response time within proximity. >> >> It's also meant to remove hosts that don't respond from the list so that >> mount attempts are not made to them (hence the need to use address). >> >> When use_hostname_for_mounts is set there will be multiple host structures >> with the same host name and each would be probed by name so all would be >> considered. >> >> But the probe is not tied to IP address so each probe failure would remove >> one of the structures from the list resulting in a reduced list of hosts >> to try to mount. >> >> So it's then quite possible a mount attempt will be made to a host that >> isn't responding with
Re: [ANNOUNCE] autofs 5.1.2 release
On Thu, Dec 21 2017, Ian Kent wrote: > On 21/12/17 19:06, Ian Kent wrote: >> On 21/12/17 09:09, NeilBrown wrote: >>> On Wed, Dec 20 2017, Ian Kent wrote: >>> On 20/12/17 13:52, Ian Kent wrote: > On 20/12/17 11:29, NeilBrown wrote: >> >> Hi Ian, >> I've been looking at: >> >>> - add configuration option to use fqdn in mounts. >> >> (commit 9aeef772604) because using this new option causes a regression. >> If you are using the "replicated server" functionality, then >> use_hostname_for_mounts = yes >> completely disables it. > > Yes, that's not quite right. > > It disables the probe and proximity check for each distinct host > name used. > > Each of the entries in the list of hosts should still be > attempted and given that NFS ping is also now used in the NFS > mount module what's lost is the preferred ordering of the hosts > list. > >> >> This is caused by: >> >> diff --git a/modules/replicated.c b/modules/replicated.c >> index 32860d5fe245..8437f5f3d5b2 100644 >> --- a/modules/replicated.c >> +++ b/modules/replicated.c >> @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host >> **list, >> if (!*list) >> return 0; >> >> + /* If we're using the host name then there's no point probing >> +* avialability and respose time. >> +*/ >> + if (defaults_use_hostname_for_mounts()) >> + return 1; >> + >> /* Use closest hosts to choose NFS version */ >> >> My question is: why what this particular change made. > > It was a while ago but there were complains about using the IP > address for mounts. It was requested to provide a way to prevent > that and force the use of the host name in mounts. > >> Why can't prune_host_list() be allowed to do it's thing >> when use_hostname_for_mounts is set. > > We could if each host name resolved to a single IP address. > > I'd need to check that use_hostname_for_mounts doesn't get > in the road but the host struct should have ->rr set to true > if it has multiple addresses so changing it to work the way > your recommending shouldn't be hard. I think there's a couple > of places that would need to be checked. > > If the host does resolve to multiple addresses the situation > is different. There's no way to stop the actual mount from > trying an IP address that's not responding and proximity > doesn't make sense either again because every time a lookup > is done on the host name (eg. at mount time) the next address > in its list will be returned which can and usually is different > from what would have been checked. > >> I understand that it would be pointless choosing between >> the different interfaces of a multi-homed host, but there is still value >> in choosing between multiple distinct hosts. >> >> What, if anything, might go wrong if I simply reverse this chunk of the >> patch? > > You'll get IP addresses in the logs in certain cases but that > should be all. > > It would probably be better to ensure that the checks are done > if the host name resolves to a single IP address. I think that should be "if the host names in the list each resolve to a single IP address", otherwise the round robin behavior would probably still get in the road. >>> >>> I cannot see why the round-robin behavior would get in the road. >>> It might be pointless to probe each IP address on a multi-homed host if >>> we are just going to mount by host name, but I don't see how it hurts. >> >> I was wondering whether I had considered the configuration option >> when setting ->rr of the host struct. I haven't so it should be set >> if the name resolves to multiple addresses. >> >>> >>> So this is what I'm thinking. Some simple testing suggests that >>> it does the right things. >>> >>> If a host has addresses with different proximity they will still be >>> probed separately, but this won't affect the final choice. >> >> Well no but ... >> >> The prune_host_list() function is meant to put the list of hosts >> in proximity order with hosts ordered by response time within proximity. >> >> It's also meant to remove hosts that don't respond from the list so that >> mount attempts are not made to them (hence the need to use address). >> >> When use_hostname_for_mounts is set there will be multiple host structures >> with the same host name and each would be probed by name so all would be >> considered. >> >> But the probe is not tied to IP address so each probe failure would remove >> one of the structures from the list resulting in a reduced list of hosts >> to try to mount. >> >> So it's then quite possible a mount attempt will be made to a host that >> isn't responding with
Re: [ANNOUNCE] autofs 5.1.2 release
On 21/12/17 19:06, Ian Kent wrote: > On 21/12/17 09:09, NeilBrown wrote: >> On Wed, Dec 20 2017, Ian Kent wrote: >> >>> On 20/12/17 13:52, Ian Kent wrote: On 20/12/17 11:29, NeilBrown wrote: > > Hi Ian, > I've been looking at: > >> - add configuration option to use fqdn in mounts. > > (commit 9aeef772604) because using this new option causes a regression. > If you are using the "replicated server" functionality, then > use_hostname_for_mounts = yes > completely disables it. Yes, that's not quite right. It disables the probe and proximity check for each distinct host name used. Each of the entries in the list of hosts should still be attempted and given that NFS ping is also now used in the NFS mount module what's lost is the preferred ordering of the hosts list. > > This is caused by: > > diff --git a/modules/replicated.c b/modules/replicated.c > index 32860d5fe245..8437f5f3d5b2 100644 > --- a/modules/replicated.c > +++ b/modules/replicated.c > @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host > **list, > if (!*list) > return 0; > > + /* If we're using the host name then there's no point probing > +* avialability and respose time. > +*/ > + if (defaults_use_hostname_for_mounts()) > + return 1; > + > /* Use closest hosts to choose NFS version */ > > My question is: why what this particular change made. It was a while ago but there were complains about using the IP address for mounts. It was requested to provide a way to prevent that and force the use of the host name in mounts. > Why can't prune_host_list() be allowed to do it's thing > when use_hostname_for_mounts is set. We could if each host name resolved to a single IP address. I'd need to check that use_hostname_for_mounts doesn't get in the road but the host struct should have ->rr set to true if it has multiple addresses so changing it to work the way your recommending shouldn't be hard. I think there's a couple of places that would need to be checked. If the host does resolve to multiple addresses the situation is different. There's no way to stop the actual mount from trying an IP address that's not responding and proximity doesn't make sense either again because every time a lookup is done on the host name (eg. at mount time) the next address in its list will be returned which can and usually is different from what would have been checked. > I understand that it would be pointless choosing between > the different interfaces of a multi-homed host, but there is still value > in choosing between multiple distinct hosts. > > What, if anything, might go wrong if I simply reverse this chunk of the > patch? You'll get IP addresses in the logs in certain cases but that should be all. It would probably be better to ensure that the checks are done if the host name resolves to a single IP address. >>> >>> I think that should be "if the host names in the list each resolve >>> to a single IP address", otherwise the round robin behavior would >>> probably still get in the road. >> >> I cannot see why the round-robin behavior would get in the road. >> It might be pointless to probe each IP address on a multi-homed host if >> we are just going to mount by host name, but I don't see how it hurts. > > I was wondering whether I had considered the configuration option > when setting ->rr of the host struct. I haven't so it should be set > if the name resolves to multiple addresses. > >> >> So this is what I'm thinking. Some simple testing suggests that >> it does the right things. >> >> If a host has addresses with different proximity they will still be >> probed separately, but this won't affect the final choice. > > Well no but ... > > The prune_host_list() function is meant to put the list of hosts > in proximity order with hosts ordered by response time within proximity. > > It's also meant to remove hosts that don't respond from the list so that > mount attempts are not made to them (hence the need to use address). > > When use_hostname_for_mounts is set there will be multiple host structures > with the same host name and each would be probed by name so all would be > considered. > > But the probe is not tied to IP address so each probe failure would remove > one of the structures from the list resulting in a reduced list of hosts > to try to mount. > > So it's then quite possible a mount attempt will be made to a host that > isn't responding with an associated long delay, around 3 minutes IIRC. > That's not good for the interactive nature of autofs. > > It's also possible that a number of the hosts in
Re: [ANNOUNCE] autofs 5.1.2 release
On 21/12/17 19:06, Ian Kent wrote: > On 21/12/17 09:09, NeilBrown wrote: >> On Wed, Dec 20 2017, Ian Kent wrote: >> >>> On 20/12/17 13:52, Ian Kent wrote: On 20/12/17 11:29, NeilBrown wrote: > > Hi Ian, > I've been looking at: > >> - add configuration option to use fqdn in mounts. > > (commit 9aeef772604) because using this new option causes a regression. > If you are using the "replicated server" functionality, then > use_hostname_for_mounts = yes > completely disables it. Yes, that's not quite right. It disables the probe and proximity check for each distinct host name used. Each of the entries in the list of hosts should still be attempted and given that NFS ping is also now used in the NFS mount module what's lost is the preferred ordering of the hosts list. > > This is caused by: > > diff --git a/modules/replicated.c b/modules/replicated.c > index 32860d5fe245..8437f5f3d5b2 100644 > --- a/modules/replicated.c > +++ b/modules/replicated.c > @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host > **list, > if (!*list) > return 0; > > + /* If we're using the host name then there's no point probing > +* avialability and respose time. > +*/ > + if (defaults_use_hostname_for_mounts()) > + return 1; > + > /* Use closest hosts to choose NFS version */ > > My question is: why what this particular change made. It was a while ago but there were complains about using the IP address for mounts. It was requested to provide a way to prevent that and force the use of the host name in mounts. > Why can't prune_host_list() be allowed to do it's thing > when use_hostname_for_mounts is set. We could if each host name resolved to a single IP address. I'd need to check that use_hostname_for_mounts doesn't get in the road but the host struct should have ->rr set to true if it has multiple addresses so changing it to work the way your recommending shouldn't be hard. I think there's a couple of places that would need to be checked. If the host does resolve to multiple addresses the situation is different. There's no way to stop the actual mount from trying an IP address that's not responding and proximity doesn't make sense either again because every time a lookup is done on the host name (eg. at mount time) the next address in its list will be returned which can and usually is different from what would have been checked. > I understand that it would be pointless choosing between > the different interfaces of a multi-homed host, but there is still value > in choosing between multiple distinct hosts. > > What, if anything, might go wrong if I simply reverse this chunk of the > patch? You'll get IP addresses in the logs in certain cases but that should be all. It would probably be better to ensure that the checks are done if the host name resolves to a single IP address. >>> >>> I think that should be "if the host names in the list each resolve >>> to a single IP address", otherwise the round robin behavior would >>> probably still get in the road. >> >> I cannot see why the round-robin behavior would get in the road. >> It might be pointless to probe each IP address on a multi-homed host if >> we are just going to mount by host name, but I don't see how it hurts. > > I was wondering whether I had considered the configuration option > when setting ->rr of the host struct. I haven't so it should be set > if the name resolves to multiple addresses. > >> >> So this is what I'm thinking. Some simple testing suggests that >> it does the right things. >> >> If a host has addresses with different proximity they will still be >> probed separately, but this won't affect the final choice. > > Well no but ... > > The prune_host_list() function is meant to put the list of hosts > in proximity order with hosts ordered by response time within proximity. > > It's also meant to remove hosts that don't respond from the list so that > mount attempts are not made to them (hence the need to use address). > > When use_hostname_for_mounts is set there will be multiple host structures > with the same host name and each would be probed by name so all would be > considered. > > But the probe is not tied to IP address so each probe failure would remove > one of the structures from the list resulting in a reduced list of hosts > to try to mount. > > So it's then quite possible a mount attempt will be made to a host that > isn't responding with an associated long delay, around 3 minutes IIRC. > That's not good for the interactive nature of autofs. > > It's also possible that a number of the hosts in
Re: [ANNOUNCE] autofs 5.1.2 release
On 21/12/17 09:09, NeilBrown wrote: > On Wed, Dec 20 2017, Ian Kent wrote: > >> On 20/12/17 13:52, Ian Kent wrote: >>> On 20/12/17 11:29, NeilBrown wrote: Hi Ian, I've been looking at: > - add configuration option to use fqdn in mounts. (commit 9aeef772604) because using this new option causes a regression. If you are using the "replicated server" functionality, then use_hostname_for_mounts = yes completely disables it. >>> >>> Yes, that's not quite right. >>> >>> It disables the probe and proximity check for each distinct host >>> name used. >>> >>> Each of the entries in the list of hosts should still be >>> attempted and given that NFS ping is also now used in the NFS >>> mount module what's lost is the preferred ordering of the hosts >>> list. >>> This is caused by: diff --git a/modules/replicated.c b/modules/replicated.c index 32860d5fe245..8437f5f3d5b2 100644 --- a/modules/replicated.c +++ b/modules/replicated.c @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host **list, if (!*list) return 0; + /* If we're using the host name then there's no point probing +* avialability and respose time. +*/ + if (defaults_use_hostname_for_mounts()) + return 1; + /* Use closest hosts to choose NFS version */ My question is: why what this particular change made. >>> >>> It was a while ago but there were complains about using the IP >>> address for mounts. It was requested to provide a way to prevent >>> that and force the use of the host name in mounts. >>> Why can't prune_host_list() be allowed to do it's thing when use_hostname_for_mounts is set. >>> >>> We could if each host name resolved to a single IP address. >>> >>> I'd need to check that use_hostname_for_mounts doesn't get >>> in the road but the host struct should have ->rr set to true >>> if it has multiple addresses so changing it to work the way >>> your recommending shouldn't be hard. I think there's a couple >>> of places that would need to be checked. >>> >>> If the host does resolve to multiple addresses the situation >>> is different. There's no way to stop the actual mount from >>> trying an IP address that's not responding and proximity >>> doesn't make sense either again because every time a lookup >>> is done on the host name (eg. at mount time) the next address >>> in its list will be returned which can and usually is different >>> from what would have been checked. >>> I understand that it would be pointless choosing between the different interfaces of a multi-homed host, but there is still value in choosing between multiple distinct hosts. What, if anything, might go wrong if I simply reverse this chunk of the patch? >>> >>> You'll get IP addresses in the logs in certain cases but that >>> should be all. >>> >>> It would probably be better to ensure that the checks are done >>> if the host name resolves to a single IP address. >> >> I think that should be "if the host names in the list each resolve >> to a single IP address", otherwise the round robin behavior would >> probably still get in the road. > > I cannot see why the round-robin behavior would get in the road. > It might be pointless to probe each IP address on a multi-homed host if > we are just going to mount by host name, but I don't see how it hurts. I was wondering whether I had considered the configuration option when setting ->rr of the host struct. I haven't so it should be set if the name resolves to multiple addresses. > > So this is what I'm thinking. Some simple testing suggests that > it does the right things. > > If a host has addresses with different proximity they will still be > probed separately, but this won't affect the final choice. Well no but ... The prune_host_list() function is meant to put the list of hosts in proximity order with hosts ordered by response time within proximity. It's also meant to remove hosts that don't respond from the list so that mount attempts are not made to them (hence the need to use address). When use_hostname_for_mounts is set there will be multiple host structures with the same host name and each would be probed by name so all would be considered. But the probe is not tied to IP address so each probe failure would remove one of the structures from the list resulting in a reduced list of hosts to try to mount. So it's then quite possible a mount attempt will be made to a host that isn't responding with an associated long delay, around 3 minutes IIRC. That's not good for the interactive nature of autofs. It's also possible that a number of the hosts in the list are not responding causing a portion of them to be removed making the list smaller and the likelihood that the mount will fail when one of the hosts actually would succeed but
Re: [ANNOUNCE] autofs 5.1.2 release
On 21/12/17 09:09, NeilBrown wrote: > On Wed, Dec 20 2017, Ian Kent wrote: > >> On 20/12/17 13:52, Ian Kent wrote: >>> On 20/12/17 11:29, NeilBrown wrote: Hi Ian, I've been looking at: > - add configuration option to use fqdn in mounts. (commit 9aeef772604) because using this new option causes a regression. If you are using the "replicated server" functionality, then use_hostname_for_mounts = yes completely disables it. >>> >>> Yes, that's not quite right. >>> >>> It disables the probe and proximity check for each distinct host >>> name used. >>> >>> Each of the entries in the list of hosts should still be >>> attempted and given that NFS ping is also now used in the NFS >>> mount module what's lost is the preferred ordering of the hosts >>> list. >>> This is caused by: diff --git a/modules/replicated.c b/modules/replicated.c index 32860d5fe245..8437f5f3d5b2 100644 --- a/modules/replicated.c +++ b/modules/replicated.c @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host **list, if (!*list) return 0; + /* If we're using the host name then there's no point probing +* avialability and respose time. +*/ + if (defaults_use_hostname_for_mounts()) + return 1; + /* Use closest hosts to choose NFS version */ My question is: why what this particular change made. >>> >>> It was a while ago but there were complains about using the IP >>> address for mounts. It was requested to provide a way to prevent >>> that and force the use of the host name in mounts. >>> Why can't prune_host_list() be allowed to do it's thing when use_hostname_for_mounts is set. >>> >>> We could if each host name resolved to a single IP address. >>> >>> I'd need to check that use_hostname_for_mounts doesn't get >>> in the road but the host struct should have ->rr set to true >>> if it has multiple addresses so changing it to work the way >>> your recommending shouldn't be hard. I think there's a couple >>> of places that would need to be checked. >>> >>> If the host does resolve to multiple addresses the situation >>> is different. There's no way to stop the actual mount from >>> trying an IP address that's not responding and proximity >>> doesn't make sense either again because every time a lookup >>> is done on the host name (eg. at mount time) the next address >>> in its list will be returned which can and usually is different >>> from what would have been checked. >>> I understand that it would be pointless choosing between the different interfaces of a multi-homed host, but there is still value in choosing between multiple distinct hosts. What, if anything, might go wrong if I simply reverse this chunk of the patch? >>> >>> You'll get IP addresses in the logs in certain cases but that >>> should be all. >>> >>> It would probably be better to ensure that the checks are done >>> if the host name resolves to a single IP address. >> >> I think that should be "if the host names in the list each resolve >> to a single IP address", otherwise the round robin behavior would >> probably still get in the road. > > I cannot see why the round-robin behavior would get in the road. > It might be pointless to probe each IP address on a multi-homed host if > we are just going to mount by host name, but I don't see how it hurts. I was wondering whether I had considered the configuration option when setting ->rr of the host struct. I haven't so it should be set if the name resolves to multiple addresses. > > So this is what I'm thinking. Some simple testing suggests that > it does the right things. > > If a host has addresses with different proximity they will still be > probed separately, but this won't affect the final choice. Well no but ... The prune_host_list() function is meant to put the list of hosts in proximity order with hosts ordered by response time within proximity. It's also meant to remove hosts that don't respond from the list so that mount attempts are not made to them (hence the need to use address). When use_hostname_for_mounts is set there will be multiple host structures with the same host name and each would be probed by name so all would be considered. But the probe is not tied to IP address so each probe failure would remove one of the structures from the list resulting in a reduced list of hosts to try to mount. So it's then quite possible a mount attempt will be made to a host that isn't responding with an associated long delay, around 3 minutes IIRC. That's not good for the interactive nature of autofs. It's also possible that a number of the hosts in the list are not responding causing a portion of them to be removed making the list smaller and the likelihood that the mount will fail when one of the hosts actually would succeed but
Re: [ANNOUNCE] autofs 5.1.2 release
On Wed, Dec 20 2017, Ian Kent wrote: > On 20/12/17 13:52, Ian Kent wrote: >> On 20/12/17 11:29, NeilBrown wrote: >>> >>> Hi Ian, >>> I've been looking at: >>> - add configuration option to use fqdn in mounts. >>> >>> (commit 9aeef772604) because using this new option causes a regression. >>> If you are using the "replicated server" functionality, then >>> use_hostname_for_mounts = yes >>> completely disables it. >> >> Yes, that's not quite right. >> >> It disables the probe and proximity check for each distinct host >> name used. >> >> Each of the entries in the list of hosts should still be >> attempted and given that NFS ping is also now used in the NFS >> mount module what's lost is the preferred ordering of the hosts >> list. >> >>> >>> This is caused by: >>> >>> diff --git a/modules/replicated.c b/modules/replicated.c >>> index 32860d5fe245..8437f5f3d5b2 100644 >>> --- a/modules/replicated.c >>> +++ b/modules/replicated.c >>> @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host >>> **list, >>> if (!*list) >>> return 0; >>> >>> + /* If we're using the host name then there's no point probing >>> +* avialability and respose time. >>> +*/ >>> + if (defaults_use_hostname_for_mounts()) >>> + return 1; >>> + >>> /* Use closest hosts to choose NFS version */ >>> >>> My question is: why what this particular change made. >> >> It was a while ago but there were complains about using the IP >> address for mounts. It was requested to provide a way to prevent >> that and force the use of the host name in mounts. >> >>> Why can't prune_host_list() be allowed to do it's thing >>> when use_hostname_for_mounts is set. >> >> We could if each host name resolved to a single IP address. >> >> I'd need to check that use_hostname_for_mounts doesn't get >> in the road but the host struct should have ->rr set to true >> if it has multiple addresses so changing it to work the way >> your recommending shouldn't be hard. I think there's a couple >> of places that would need to be checked. >> >> If the host does resolve to multiple addresses the situation >> is different. There's no way to stop the actual mount from >> trying an IP address that's not responding and proximity >> doesn't make sense either again because every time a lookup >> is done on the host name (eg. at mount time) the next address >> in its list will be returned which can and usually is different >> from what would have been checked. >> >>> I understand that it would be pointless choosing between >>> the different interfaces of a multi-homed host, but there is still value >>> in choosing between multiple distinct hosts. >>> >>> What, if anything, might go wrong if I simply reverse this chunk of the >>> patch? >> >> You'll get IP addresses in the logs in certain cases but that >> should be all. >> >> It would probably be better to ensure that the checks are done >> if the host name resolves to a single IP address. > > I think that should be "if the host names in the list each resolve > to a single IP address", otherwise the round robin behavior would > probably still get in the road. I cannot see why the round-robin behavior would get in the road. It might be pointless to probe each IP address on a multi-homed host if we are just going to mount by host name, but I don't see how it hurts. So this is what I'm thinking. Some simple testing suggests that it does the right things. If a host has addresses with different proximity they will still be probed separately, but this won't affect the final choice. Thanks, NeilBrown 8<--- Subject: use_hostname_for_mounts shouldn't prevent selection among replica If several replicas have been specified for a mount point, and use_hostname_for_mount is set to "yes", the selection between these replicas is currently disabled and the last in the list is always chosen. There is little point selecting between different interfaces on the one host in this case, but it is still worth selecting between different hosts, particularly if different weights have been specified. This patch restores the "prune_host_list()" functionality when use_hostname_for_mount is set, and modifies it slightly so that once an IP address with a given proximity has been successfully probed, other IP address for the same host(weight):/path and proximity are ignored. Signed-off-by: NeilBrowndiff --git a/modules/replicated.c b/modules/replicated.c index 3ac4c70f4062..16cf873513ff 100644 --- a/modules/replicated.c +++ b/modules/replicated.c @@ -714,7 +714,7 @@ done: int prune_host_list(unsigned logopt, struct host **list, unsigned int vers, int port) { - struct host *this, *last, *first; + struct host *this, *last, *first, *prev; struct host *new = NULL; unsigned int proximity, selected_version = 0; unsigned int v2_tcp_count, v3_tcp_count,
Re: [ANNOUNCE] autofs 5.1.2 release
On Wed, Dec 20 2017, Ian Kent wrote: > On 20/12/17 13:52, Ian Kent wrote: >> On 20/12/17 11:29, NeilBrown wrote: >>> >>> Hi Ian, >>> I've been looking at: >>> - add configuration option to use fqdn in mounts. >>> >>> (commit 9aeef772604) because using this new option causes a regression. >>> If you are using the "replicated server" functionality, then >>> use_hostname_for_mounts = yes >>> completely disables it. >> >> Yes, that's not quite right. >> >> It disables the probe and proximity check for each distinct host >> name used. >> >> Each of the entries in the list of hosts should still be >> attempted and given that NFS ping is also now used in the NFS >> mount module what's lost is the preferred ordering of the hosts >> list. >> >>> >>> This is caused by: >>> >>> diff --git a/modules/replicated.c b/modules/replicated.c >>> index 32860d5fe245..8437f5f3d5b2 100644 >>> --- a/modules/replicated.c >>> +++ b/modules/replicated.c >>> @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host >>> **list, >>> if (!*list) >>> return 0; >>> >>> + /* If we're using the host name then there's no point probing >>> +* avialability and respose time. >>> +*/ >>> + if (defaults_use_hostname_for_mounts()) >>> + return 1; >>> + >>> /* Use closest hosts to choose NFS version */ >>> >>> My question is: why what this particular change made. >> >> It was a while ago but there were complains about using the IP >> address for mounts. It was requested to provide a way to prevent >> that and force the use of the host name in mounts. >> >>> Why can't prune_host_list() be allowed to do it's thing >>> when use_hostname_for_mounts is set. >> >> We could if each host name resolved to a single IP address. >> >> I'd need to check that use_hostname_for_mounts doesn't get >> in the road but the host struct should have ->rr set to true >> if it has multiple addresses so changing it to work the way >> your recommending shouldn't be hard. I think there's a couple >> of places that would need to be checked. >> >> If the host does resolve to multiple addresses the situation >> is different. There's no way to stop the actual mount from >> trying an IP address that's not responding and proximity >> doesn't make sense either again because every time a lookup >> is done on the host name (eg. at mount time) the next address >> in its list will be returned which can and usually is different >> from what would have been checked. >> >>> I understand that it would be pointless choosing between >>> the different interfaces of a multi-homed host, but there is still value >>> in choosing between multiple distinct hosts. >>> >>> What, if anything, might go wrong if I simply reverse this chunk of the >>> patch? >> >> You'll get IP addresses in the logs in certain cases but that >> should be all. >> >> It would probably be better to ensure that the checks are done >> if the host name resolves to a single IP address. > > I think that should be "if the host names in the list each resolve > to a single IP address", otherwise the round robin behavior would > probably still get in the road. I cannot see why the round-robin behavior would get in the road. It might be pointless to probe each IP address on a multi-homed host if we are just going to mount by host name, but I don't see how it hurts. So this is what I'm thinking. Some simple testing suggests that it does the right things. If a host has addresses with different proximity they will still be probed separately, but this won't affect the final choice. Thanks, NeilBrown 8<--- Subject: use_hostname_for_mounts shouldn't prevent selection among replica If several replicas have been specified for a mount point, and use_hostname_for_mount is set to "yes", the selection between these replicas is currently disabled and the last in the list is always chosen. There is little point selecting between different interfaces on the one host in this case, but it is still worth selecting between different hosts, particularly if different weights have been specified. This patch restores the "prune_host_list()" functionality when use_hostname_for_mount is set, and modifies it slightly so that once an IP address with a given proximity has been successfully probed, other IP address for the same host(weight):/path and proximity are ignored. Signed-off-by: NeilBrown diff --git a/modules/replicated.c b/modules/replicated.c index 3ac4c70f4062..16cf873513ff 100644 --- a/modules/replicated.c +++ b/modules/replicated.c @@ -714,7 +714,7 @@ done: int prune_host_list(unsigned logopt, struct host **list, unsigned int vers, int port) { - struct host *this, *last, *first; + struct host *this, *last, *first, *prev; struct host *new = NULL; unsigned int proximity, selected_version = 0; unsigned int v2_tcp_count, v3_tcp_count, v4_tcp_count; @@
Re: [ANNOUNCE] autofs 5.1.2 release
On 20/12/17 13:52, Ian Kent wrote: > On 20/12/17 11:29, NeilBrown wrote: >> >> Hi Ian, >> I've been looking at: >> >>> - add configuration option to use fqdn in mounts. >> >> (commit 9aeef772604) because using this new option causes a regression. >> If you are using the "replicated server" functionality, then >> use_hostname_for_mounts = yes >> completely disables it. > > Yes, that's not quite right. > > It disables the probe and proximity check for each distinct host > name used. > > Each of the entries in the list of hosts should still be > attempted and given that NFS ping is also now used in the NFS > mount module what's lost is the preferred ordering of the hosts > list. Mmm that's also not right. An NFS ping is only done on failed local bind mount to check the NFS server is running on the local machine. So that availability check needs to be done at mount time if the proximity check is not done Ian
Re: [ANNOUNCE] autofs 5.1.2 release
On 20/12/17 13:52, Ian Kent wrote: > On 20/12/17 11:29, NeilBrown wrote: >> >> Hi Ian, >> I've been looking at: >> >>> - add configuration option to use fqdn in mounts. >> >> (commit 9aeef772604) because using this new option causes a regression. >> If you are using the "replicated server" functionality, then >> use_hostname_for_mounts = yes >> completely disables it. > > Yes, that's not quite right. > > It disables the probe and proximity check for each distinct host > name used. > > Each of the entries in the list of hosts should still be > attempted and given that NFS ping is also now used in the NFS > mount module what's lost is the preferred ordering of the hosts > list. Mmm that's also not right. An NFS ping is only done on failed local bind mount to check the NFS server is running on the local machine. So that availability check needs to be done at mount time if the proximity check is not done Ian
Re: [ANNOUNCE] autofs 5.1.2 release
On 20/12/17 14:10, Ian Kent wrote: > On 20/12/17 13:52, Ian Kent wrote: >> On 20/12/17 11:29, NeilBrown wrote: >>> >>> Hi Ian, >>> I've been looking at: >>> - add configuration option to use fqdn in mounts. >>> >>> (commit 9aeef772604) because using this new option causes a regression. >>> If you are using the "replicated server" functionality, then >>> use_hostname_for_mounts = yes >>> completely disables it. >> >> Yes, that's not quite right. >> >> It disables the probe and proximity check for each distinct host >> name used. >> >> Each of the entries in the list of hosts should still be >> attempted and given that NFS ping is also now used in the NFS >> mount module what's lost is the preferred ordering of the hosts >> list. >> >>> >>> This is caused by: >>> >>> diff --git a/modules/replicated.c b/modules/replicated.c >>> index 32860d5fe245..8437f5f3d5b2 100644 >>> --- a/modules/replicated.c >>> +++ b/modules/replicated.c >>> @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host >>> **list, >>> if (!*list) >>> return 0; >>> >>> + /* If we're using the host name then there's no point probing >>> +* avialability and respose time. >>> +*/ >>> + if (defaults_use_hostname_for_mounts()) >>> + return 1; >>> + >>> /* Use closest hosts to choose NFS version */ >>> >>> My question is: why what this particular change made. >> >> It was a while ago but there were complains about using the IP >> address for mounts. It was requested to provide a way to prevent >> that and force the use of the host name in mounts. >> >>> Why can't prune_host_list() be allowed to do it's thing >>> when use_hostname_for_mounts is set. >> >> We could if each host name resolved to a single IP address. >> >> I'd need to check that use_hostname_for_mounts doesn't get >> in the road but the host struct should have ->rr set to true >> if it has multiple addresses so changing it to work the way >> your recommending shouldn't be hard. I think there's a couple >> of places that would need to be checked. >> >> If the host does resolve to multiple addresses the situation >> is different. There's no way to stop the actual mount from >> trying an IP address that's not responding and proximity >> doesn't make sense either again because every time a lookup >> is done on the host name (eg. at mount time) the next address >> in its list will be returned which can and usually is different >> from what would have been checked. >> >>> I understand that it would be pointless choosing between >>> the different interfaces of a multi-homed host, but there is still value >>> in choosing between multiple distinct hosts. >>> >>> What, if anything, might go wrong if I simply reverse this chunk of the >>> patch? >> >> You'll get IP addresses in the logs in certain cases but that >> should be all. >> >> It would probably be better to ensure that the checks are done >> if the host name resolves to a single IP address. > > I think that should be "if the host names in the list each resolve > to a single IP address", otherwise the round robin behavior would > probably still get in the road. I think maybe this is sufficient autofs-5.1.4 - use proximity check if all host names are simple From: Ian KentCurrently if the configuration option use_hostname_for_mounts is set then the proximity calcualtion is not done for the list of hosts. But if each host name in the host list resolves to a single IP address then performing the proximity check still makes sense. Signed-off-by: Ian Kent --- modules/replicated.c | 32 ++-- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/modules/replicated.c b/modules/replicated.c index 3ac4c70f..e5c2276d 100644 --- a/modules/replicated.c +++ b/modules/replicated.c @@ -711,6 +711,24 @@ done: return 0; } +static unsigned int is_hosts_list_simple(struct host *list) +{ + struct host *this = list; + unsigned int ret = 1; + + while (this) { + struct host *next = this->next; + + if (this->rr) { + ret = 0; + break; + } + this = next; + } + + return ret; +} + int prune_host_list(unsigned logopt, struct host **list, unsigned int vers, int port) { @@ -726,12 +744,6 @@ int prune_host_list(unsigned logopt, struct host **list, if (!*list) return 0; - /* If we're using the host name then there's no point probing -* avialability and respose time. -*/ - if (defaults_use_hostname_for_mounts()) - return 1; - /* Use closest hosts to choose NFS version */ first = *list; @@ -767,6 +779,14 @@ int prune_host_list(unsigned logopt, struct host **list, return 1; } + /* If we're using the host
Re: [ANNOUNCE] autofs 5.1.2 release
On 20/12/17 14:10, Ian Kent wrote: > On 20/12/17 13:52, Ian Kent wrote: >> On 20/12/17 11:29, NeilBrown wrote: >>> >>> Hi Ian, >>> I've been looking at: >>> - add configuration option to use fqdn in mounts. >>> >>> (commit 9aeef772604) because using this new option causes a regression. >>> If you are using the "replicated server" functionality, then >>> use_hostname_for_mounts = yes >>> completely disables it. >> >> Yes, that's not quite right. >> >> It disables the probe and proximity check for each distinct host >> name used. >> >> Each of the entries in the list of hosts should still be >> attempted and given that NFS ping is also now used in the NFS >> mount module what's lost is the preferred ordering of the hosts >> list. >> >>> >>> This is caused by: >>> >>> diff --git a/modules/replicated.c b/modules/replicated.c >>> index 32860d5fe245..8437f5f3d5b2 100644 >>> --- a/modules/replicated.c >>> +++ b/modules/replicated.c >>> @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host >>> **list, >>> if (!*list) >>> return 0; >>> >>> + /* If we're using the host name then there's no point probing >>> +* avialability and respose time. >>> +*/ >>> + if (defaults_use_hostname_for_mounts()) >>> + return 1; >>> + >>> /* Use closest hosts to choose NFS version */ >>> >>> My question is: why what this particular change made. >> >> It was a while ago but there were complains about using the IP >> address for mounts. It was requested to provide a way to prevent >> that and force the use of the host name in mounts. >> >>> Why can't prune_host_list() be allowed to do it's thing >>> when use_hostname_for_mounts is set. >> >> We could if each host name resolved to a single IP address. >> >> I'd need to check that use_hostname_for_mounts doesn't get >> in the road but the host struct should have ->rr set to true >> if it has multiple addresses so changing it to work the way >> your recommending shouldn't be hard. I think there's a couple >> of places that would need to be checked. >> >> If the host does resolve to multiple addresses the situation >> is different. There's no way to stop the actual mount from >> trying an IP address that's not responding and proximity >> doesn't make sense either again because every time a lookup >> is done on the host name (eg. at mount time) the next address >> in its list will be returned which can and usually is different >> from what would have been checked. >> >>> I understand that it would be pointless choosing between >>> the different interfaces of a multi-homed host, but there is still value >>> in choosing between multiple distinct hosts. >>> >>> What, if anything, might go wrong if I simply reverse this chunk of the >>> patch? >> >> You'll get IP addresses in the logs in certain cases but that >> should be all. >> >> It would probably be better to ensure that the checks are done >> if the host name resolves to a single IP address. > > I think that should be "if the host names in the list each resolve > to a single IP address", otherwise the round robin behavior would > probably still get in the road. I think maybe this is sufficient autofs-5.1.4 - use proximity check if all host names are simple From: Ian Kent Currently if the configuration option use_hostname_for_mounts is set then the proximity calcualtion is not done for the list of hosts. But if each host name in the host list resolves to a single IP address then performing the proximity check still makes sense. Signed-off-by: Ian Kent --- modules/replicated.c | 32 ++-- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/modules/replicated.c b/modules/replicated.c index 3ac4c70f..e5c2276d 100644 --- a/modules/replicated.c +++ b/modules/replicated.c @@ -711,6 +711,24 @@ done: return 0; } +static unsigned int is_hosts_list_simple(struct host *list) +{ + struct host *this = list; + unsigned int ret = 1; + + while (this) { + struct host *next = this->next; + + if (this->rr) { + ret = 0; + break; + } + this = next; + } + + return ret; +} + int prune_host_list(unsigned logopt, struct host **list, unsigned int vers, int port) { @@ -726,12 +744,6 @@ int prune_host_list(unsigned logopt, struct host **list, if (!*list) return 0; - /* If we're using the host name then there's no point probing -* avialability and respose time. -*/ - if (defaults_use_hostname_for_mounts()) - return 1; - /* Use closest hosts to choose NFS version */ first = *list; @@ -767,6 +779,14 @@ int prune_host_list(unsigned logopt, struct host **list, return 1; } + /* If we're using the host name then there's no point probing +
Re: [ANNOUNCE] autofs 5.1.2 release
On 20/12/17 13:52, Ian Kent wrote: > On 20/12/17 11:29, NeilBrown wrote: >> >> Hi Ian, >> I've been looking at: >> >>> - add configuration option to use fqdn in mounts. >> >> (commit 9aeef772604) because using this new option causes a regression. >> If you are using the "replicated server" functionality, then >> use_hostname_for_mounts = yes >> completely disables it. > > Yes, that's not quite right. > > It disables the probe and proximity check for each distinct host > name used. > > Each of the entries in the list of hosts should still be > attempted and given that NFS ping is also now used in the NFS > mount module what's lost is the preferred ordering of the hosts > list. > >> >> This is caused by: >> >> diff --git a/modules/replicated.c b/modules/replicated.c >> index 32860d5fe245..8437f5f3d5b2 100644 >> --- a/modules/replicated.c >> +++ b/modules/replicated.c >> @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host **list, >> if (!*list) >> return 0; >> >> + /* If we're using the host name then there's no point probing >> +* avialability and respose time. >> +*/ >> + if (defaults_use_hostname_for_mounts()) >> + return 1; >> + >> /* Use closest hosts to choose NFS version */ >> >> My question is: why what this particular change made. > > It was a while ago but there were complains about using the IP > address for mounts. It was requested to provide a way to prevent > that and force the use of the host name in mounts. > >> Why can't prune_host_list() be allowed to do it's thing >> when use_hostname_for_mounts is set. > > We could if each host name resolved to a single IP address. > > I'd need to check that use_hostname_for_mounts doesn't get > in the road but the host struct should have ->rr set to true > if it has multiple addresses so changing it to work the way > your recommending shouldn't be hard. I think there's a couple > of places that would need to be checked. > > If the host does resolve to multiple addresses the situation > is different. There's no way to stop the actual mount from > trying an IP address that's not responding and proximity > doesn't make sense either again because every time a lookup > is done on the host name (eg. at mount time) the next address > in its list will be returned which can and usually is different > from what would have been checked. > >> I understand that it would be pointless choosing between >> the different interfaces of a multi-homed host, but there is still value >> in choosing between multiple distinct hosts. >> >> What, if anything, might go wrong if I simply reverse this chunk of the >> patch? > > You'll get IP addresses in the logs in certain cases but that > should be all. > > It would probably be better to ensure that the checks are done > if the host name resolves to a single IP address. I think that should be "if the host names in the list each resolve to a single IP address", otherwise the round robin behavior would probably still get in the road. Ian
Re: [ANNOUNCE] autofs 5.1.2 release
On 20/12/17 13:52, Ian Kent wrote: > On 20/12/17 11:29, NeilBrown wrote: >> >> Hi Ian, >> I've been looking at: >> >>> - add configuration option to use fqdn in mounts. >> >> (commit 9aeef772604) because using this new option causes a regression. >> If you are using the "replicated server" functionality, then >> use_hostname_for_mounts = yes >> completely disables it. > > Yes, that's not quite right. > > It disables the probe and proximity check for each distinct host > name used. > > Each of the entries in the list of hosts should still be > attempted and given that NFS ping is also now used in the NFS > mount module what's lost is the preferred ordering of the hosts > list. > >> >> This is caused by: >> >> diff --git a/modules/replicated.c b/modules/replicated.c >> index 32860d5fe245..8437f5f3d5b2 100644 >> --- a/modules/replicated.c >> +++ b/modules/replicated.c >> @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host **list, >> if (!*list) >> return 0; >> >> + /* If we're using the host name then there's no point probing >> +* avialability and respose time. >> +*/ >> + if (defaults_use_hostname_for_mounts()) >> + return 1; >> + >> /* Use closest hosts to choose NFS version */ >> >> My question is: why what this particular change made. > > It was a while ago but there were complains about using the IP > address for mounts. It was requested to provide a way to prevent > that and force the use of the host name in mounts. > >> Why can't prune_host_list() be allowed to do it's thing >> when use_hostname_for_mounts is set. > > We could if each host name resolved to a single IP address. > > I'd need to check that use_hostname_for_mounts doesn't get > in the road but the host struct should have ->rr set to true > if it has multiple addresses so changing it to work the way > your recommending shouldn't be hard. I think there's a couple > of places that would need to be checked. > > If the host does resolve to multiple addresses the situation > is different. There's no way to stop the actual mount from > trying an IP address that's not responding and proximity > doesn't make sense either again because every time a lookup > is done on the host name (eg. at mount time) the next address > in its list will be returned which can and usually is different > from what would have been checked. > >> I understand that it would be pointless choosing between >> the different interfaces of a multi-homed host, but there is still value >> in choosing between multiple distinct hosts. >> >> What, if anything, might go wrong if I simply reverse this chunk of the >> patch? > > You'll get IP addresses in the logs in certain cases but that > should be all. > > It would probably be better to ensure that the checks are done > if the host name resolves to a single IP address. I think that should be "if the host names in the list each resolve to a single IP address", otherwise the round robin behavior would probably still get in the road. Ian
Re: [ANNOUNCE] autofs 5.1.2 release
On 20/12/17 11:29, NeilBrown wrote: > > Hi Ian, > I've been looking at: > >> - add configuration option to use fqdn in mounts. > > (commit 9aeef772604) because using this new option causes a regression. > If you are using the "replicated server" functionality, then > use_hostname_for_mounts = yes > completely disables it. Yes, that's not quite right. It disables the probe and proximity check for each distinct host name used. Each of the entries in the list of hosts should still be attempted and given that NFS ping is also now used in the NFS mount module what's lost is the preferred ordering of the hosts list. > > This is caused by: > > diff --git a/modules/replicated.c b/modules/replicated.c > index 32860d5fe245..8437f5f3d5b2 100644 > --- a/modules/replicated.c > +++ b/modules/replicated.c > @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host **list, > if (!*list) > return 0; > > + /* If we're using the host name then there's no point probing > +* avialability and respose time. > +*/ > + if (defaults_use_hostname_for_mounts()) > + return 1; > + > /* Use closest hosts to choose NFS version */ > > My question is: why what this particular change made. It was a while ago but there were complains about using the IP address for mounts. It was requested to provide a way to prevent that and force the use of the host name in mounts. > Why can't prune_host_list() be allowed to do it's thing > when use_hostname_for_mounts is set. We could if each host name resolved to a single IP address. I'd need to check that use_hostname_for_mounts doesn't get in the road but the host struct should have ->rr set to true if it has multiple addresses so changing it to work the way your recommending shouldn't be hard. I think there's a couple of places that would need to be checked. If the host does resolve to multiple addresses the situation is different. There's no way to stop the actual mount from trying an IP address that's not responding and proximity doesn't make sense either again because every time a lookup is done on the host name (eg. at mount time) the next address in its list will be returned which can and usually is different from what would have been checked. > I understand that it would be pointless choosing between > the different interfaces of a multi-homed host, but there is still value > in choosing between multiple distinct hosts. > > What, if anything, might go wrong if I simply reverse this chunk of the > patch? You'll get IP addresses in the logs in certain cases but that should be all. It would probably be better to ensure that the checks are done if the host name resolves to a single IP address. Ian
Re: [ANNOUNCE] autofs 5.1.2 release
On 20/12/17 11:29, NeilBrown wrote: > > Hi Ian, > I've been looking at: > >> - add configuration option to use fqdn in mounts. > > (commit 9aeef772604) because using this new option causes a regression. > If you are using the "replicated server" functionality, then > use_hostname_for_mounts = yes > completely disables it. Yes, that's not quite right. It disables the probe and proximity check for each distinct host name used. Each of the entries in the list of hosts should still be attempted and given that NFS ping is also now used in the NFS mount module what's lost is the preferred ordering of the hosts list. > > This is caused by: > > diff --git a/modules/replicated.c b/modules/replicated.c > index 32860d5fe245..8437f5f3d5b2 100644 > --- a/modules/replicated.c > +++ b/modules/replicated.c > @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host **list, > if (!*list) > return 0; > > + /* If we're using the host name then there's no point probing > +* avialability and respose time. > +*/ > + if (defaults_use_hostname_for_mounts()) > + return 1; > + > /* Use closest hosts to choose NFS version */ > > My question is: why what this particular change made. It was a while ago but there were complains about using the IP address for mounts. It was requested to provide a way to prevent that and force the use of the host name in mounts. > Why can't prune_host_list() be allowed to do it's thing > when use_hostname_for_mounts is set. We could if each host name resolved to a single IP address. I'd need to check that use_hostname_for_mounts doesn't get in the road but the host struct should have ->rr set to true if it has multiple addresses so changing it to work the way your recommending shouldn't be hard. I think there's a couple of places that would need to be checked. If the host does resolve to multiple addresses the situation is different. There's no way to stop the actual mount from trying an IP address that's not responding and proximity doesn't make sense either again because every time a lookup is done on the host name (eg. at mount time) the next address in its list will be returned which can and usually is different from what would have been checked. > I understand that it would be pointless choosing between > the different interfaces of a multi-homed host, but there is still value > in choosing between multiple distinct hosts. > > What, if anything, might go wrong if I simply reverse this chunk of the > patch? You'll get IP addresses in the logs in certain cases but that should be all. It would probably be better to ensure that the checks are done if the host name resolves to a single IP address. Ian
Re: [ANNOUNCE] autofs 5.1.2 release
Hi Ian, I've been looking at: > - add configuration option to use fqdn in mounts. (commit 9aeef772604) because using this new option causes a regression. If you are using the "replicated server" functionality, then use_hostname_for_mounts = yes completely disables it. This is caused by: diff --git a/modules/replicated.c b/modules/replicated.c index 32860d5fe245..8437f5f3d5b2 100644 --- a/modules/replicated.c +++ b/modules/replicated.c @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host **list, if (!*list) return 0; + /* If we're using the host name then there's no point probing +* avialability and respose time. +*/ + if (defaults_use_hostname_for_mounts()) + return 1; + /* Use closest hosts to choose NFS version */ My question is: why what this particular change made. Why can't prune_host_list() be allowed to do it's thing when use_hostname_for_mounts is set. I understand that it would be pointless choosing between the different interfaces of a multi-homed host, but there is still value in choosing between multiple distinct hosts. What, if anything, might go wrong if I simply reverse this chunk of the patch? Thanks, NeilBrown signature.asc Description: PGP signature
Re: [ANNOUNCE] autofs 5.1.2 release
Hi Ian, I've been looking at: > - add configuration option to use fqdn in mounts. (commit 9aeef772604) because using this new option causes a regression. If you are using the "replicated server" functionality, then use_hostname_for_mounts = yes completely disables it. This is caused by: diff --git a/modules/replicated.c b/modules/replicated.c index 32860d5fe245..8437f5f3d5b2 100644 --- a/modules/replicated.c +++ b/modules/replicated.c @@ -667,6 +667,12 @@ int prune_host_list(unsigned logopt, struct host **list, if (!*list) return 0; + /* If we're using the host name then there's no point probing +* avialability and respose time. +*/ + if (defaults_use_hostname_for_mounts()) + return 1; + /* Use closest hosts to choose NFS version */ My question is: why what this particular change made. Why can't prune_host_list() be allowed to do it's thing when use_hostname_for_mounts is set. I understand that it would be pointless choosing between the different interfaces of a multi-homed host, but there is still value in choosing between multiple distinct hosts. What, if anything, might go wrong if I simply reverse this chunk of the patch? Thanks, NeilBrown signature.asc Description: PGP signature