fix(linux_dns_issue): Re-trying DNS lookup of instance name after appending the domain from AG listener

[Aditya Niraula (AdityaNPL)]

Dns.GetHostEntry(instance) fails intermittently on linux platforms if instance doesn't have the full domain

[Ryan Clare (Kaneraz)]

Some odd spacing showing up in the diff, but it looks like it's doing what we set out to do.

[Alex Irion (alexirion10)]

Code looks ok... but this is tricky.
I'm not certain that instanceName.Contains('.') means that it's a FQDN necessarily
Did you test? How did you test? Can you provide some sample I/O?

[Aditya Niraula (AdityaNPL)]

I'm not sure if there is another way to check? I thought that if it has at least one . it could already be a FQDN?
Do we want to ignore if it is or not and just blindly append the later part of the domain from the hostDomain to the instance name extracted from instanceName?

i.e. if hostDomain = "abc.def.ghi" and instanceName="uvw.xyz" we do dns lookup on uvw.def.ghi ??

[Alex Irion (alexirion10)]

Exactly...
I don't have an answer to that. I'm asking if you've tried and thought through all of these possibilities

[Aditya Niraula (AdityaNPL)]

Alex Irion (@alexirion10) checked where this function is being called from and it looks like the only values being passed in are the replica instance names - which won't be FQDN
Have removed the check as I feel we don't need it anymore

Also renamed a few variables as I am understanding the terminologies and tech stack a little better now 😄

What do you think?

[Ryan Clare (Kaneraz)]

Aditya Niraula (@AdityaNPL) if we're concerned about getting the FQDN instead of just a hostname we can attempt to resolve the hostname first and if it fails, then attempt to append from the original listener name.

[Aditya Niraula (AdityaNPL)]

Changing this to Draft as I think this needs much more refactoring before it is ready to release publicly.
Leaving PR open to continue with reviews and suggestions.

Sure Ryan Clare (@Kaneraz) , yes I think changing to a try-catch might be less error prone for us right now - will change to that

[Aditya Niraula (AdityaNPL)]

Have refactored and added tests for the utility functions - I think should be ready for release
Alex Irion (@alexirion10) / Ryan Clare (@Kaneraz) can you pls give it a look over and let me know if you have any suggestions

[Alex Irion (alexirion10)]

I mean... I'm sure it works, but man this code is messy and fragile. I'm nervous with the amount of string parsing happening in here

[Aditya Niraula (AdityaNPL)]

The code logic of our fix:

1. Strip off any ports/named instances from both the listener and the instance
2. Try to resolve just the instance name to a FQDN
3. If it fails (as it does sometimes on linux) we append the domain fragments from the listener to the instance name (to make it a complete domain) and then re-try with the full instance domain
4. To the resolved FQDN, add back a port or a named instance (if available from step 1)

Alex Irion (@alexirion10) I agree, but looking above at the steps we need to do, I think the string manipulations are required.
I did have a look at using some in-built c# classes to perform these actions as well (like UriBuilder and SqlConnectionStringBuilder) but couldn't find any that could

---

I have some suggestions that could give us more confidence:

• I can intensify testing a bit more?
  • For example: refactor the string manipulation in ResolveDnsHostNameForInstance() into its own function and write some tests for that?
    (but it might require creation of more functions, and possibly more classes, to enable testing - which may make it more confusing)
• We can also add some string format checks to make sure inputs are in correct format before we start splitting and slicing?
  (but then we may be restricting users on the input types that they can provide)
• We could go down the path of regex - but I opted not to because it is usually more complex to develop and felt it might introduce more bugs for something we can do with some simple string manipulations (which are quicker to implement and easier to follow)

What are your thoughts?

[Ryan Clare (Kaneraz)]

As we've discussed this is a bit of a tangle, but after looking at it a few time I'm not seeing a clear way to untangle it. Once we get things in place to resolve WINS on the containers this runs in, we can consider removing the hack.

Thanks for the unit tests, they give me confidence that this is going to remain stable as we keep moving forward and balances out my concerns with how tangled it feels.