On 04/11/2011 06:07 PM, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 04/11/2011 07:01 AM, Jakub Hrozek wrote:
> The first patch fixes
https://bugzilla.redhat.com/show_bug.cgi?id=694444
> by not detecting duplicates at all.
Ack.
> The second patch is an optimization. Since it is possible that the list
> of servers contains duplicates now, the code marks all duplicates when
> setting a server status, too. I'm submitting it as a separate patch
> because it might be easier to push only one of them if QE thinks they
> have the capacity to test only one of them for the stable 1.5 branch.
Nack.
While it's not as if we call this function often, it seems
counter-intuitive to me that we'd be looping through all the server
lists (which could be arbitrarily long, if we're using SRV records) on
every state update, rather than just eating this cost once when adding
new servers to the list.
In other words, we should do a duplicate check before calling
create_fo_server() and skip creating it if the name and port matches.
I actually think it is more convenient to allow duplicates than to
detect them on create_fo_server() time and disallow them. Let me explain:
It is quite possible (and actually the default for ipa-client-install)
to have the same server name as a result of a SRV query and a hard coded
hostname. The problem is the ordering of servers.
Consider that srv query resolves to "ldap.example.com,
ldap2.example.com" and the config file contains "_srv_,
ldap.example.com". If we discard duplicates after the SRV query is
resolved, the order would be "ldap2.example.com, ldap.example.com" which
is wrong.
We could reorder the list and then, when the SRV query expires, reorder
it back again, but I suspect that it is quite a bit more work and I
would prefer this to be tracked as a separate issue in 1.6 or 1.7.
Jakub