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

Revert misleading change to documentation #1576

Merged

Conversation

pseidel-kcf
Copy link
Contributor

I believe the change from this old 2018 PR is incorrect -- at least with today's behavior.

To determine this revert is required I setup two Redis instances and ran "SET environment first" on one and "SET environment second" on the other. I used the computer's hostfile to control which IP my DnsEndpoint resolved to. I briefly disconnected my network to trigger reconnection code.

 while (true) {
                try {
                    var environment = await connection.GetDatabase().StringGetAsync("environment");
                    programLogger.LogInformation(environment);
                } catch (Exception) {
                    // squelch
                } finally {
                    await Task.Delay(TimeSpan.FromSeconds(5));
                }
            }

Here are the behaviors I see if I use a DnsEndpoint and the IPs for the Redis instance change with ResolveDns set to true:

// host file points to "first" instance
[01:37:52 INF] first
[01:37:57 INF] first
[01:38:02 INF] first
[01:38:07 INF] first
// disconnect network, change host file to point to "second" instance
[01:39:01 INF] first
[01:39:06 INF] first
[01:39:11 INF] first

Here are the behaviors I see if I use a DnsEndpoint and the IPs for the Redis instance change with ResolveDns set to false:

// host file points to "first" instance
[01:42:49 INF] first
[01:42:54 INF] first
[01:42:59 INF] first
[01:43:04 INF] first
// disconnect network, change host file to point to "second" instance
[01:44:04 INF] second
[01:44:09 INF] second
[01:44:14 INF] second
[01:44:19 INF] second

@mgravell
Copy link
Collaborator

I'm happy to re-evaluate this, but it seems more like there's a bug in that something that should be working: isn't. Merging for now, if the documentation is incorrect for the current state, but the real fix here (IMO) is "make it work". That's a bigger task, though!

@mgravell mgravell merged commit 64e5b13 into StackExchange:main Oct 21, 2020
@pseidel-kcf pseidel-kcf deleted the revert-incorrect-documentation branch October 21, 2020 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants