Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

LeaderURI: self identify, avoid infinite forwarding #792

Merged
merged 3 commits into from
Feb 12, 2019

Conversation

shlomi-noach
Copy link
Collaborator

Fixes #696

In #696, a demoted leader tries to reverse proxy HTTP requests to the leader, but the LeaderURI it has is its own, which leads to infinite reverse-proxy to itself.

In this PR:

  • A node self-identifies its own URI.
  • And can check whether the LeaderURI actually points to itself.
  • And avoids an infinite loop in reverse-proxy.

The change of behavior is:

Instead of an infinite loop, the host will attempt to serve the request. This isn't ideal either: the node is not the leader, but can't figure out who the leader is (yet). A client that attempts to run an operation during this time will get a "read only" error.

Thank you to @mia0x75 @makmanalp for illustrating the problem. Can you please test this PR?

@shlomi-noach shlomi-noach mentioned this pull request Feb 6, 2019
@makmanalp
Copy link

@shlomi-noach Thanks a billion for being on this so quickly. I'll test the fix first thing today.

@makmanalp
Copy link

Hi Shlomi, apologies, took me a while to set up our custom orchestrator build, but I now have the docker image with your patch backported onto v3.0.10 (what we use), tested it yesterday and overnight in our QA environment and so far it looks good. We're letting it sit until Monday until we're comfortable pushing it to production but the issue seems to have been fixed - memory usage is quite stable and restarting or killing the pods doesn't seem to trigger the problem.

@shlomi-noach
Copy link
Collaborator Author

Thanks for the update. If all goes well I'll merge it this week.

@makmanalp
Copy link

@shlomi-noach just deployed it in production, looks great! Thanks again for the speedy fix!

@shlomi-noach shlomi-noach merged commit 88076ab into master Feb 12, 2019
@shlomi-noach shlomi-noach deleted the leader-uri-infinite-loop-fix branch February 12, 2019 04:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants