remove ServerDiscoverySelector i.e. hardcoded zk dependency from DruidLeaderClient

[himanshug]

towards #9053

Brings back #9481 that was reverted in #9702 because bug #9701 was discovered.

#9701 root cause was really same as that of #8716, both were fixed in #9717

TLDR:
Change in this patch was introduced in the past, an issue was discovered whose root cause was ideally fixed by another change. That couldn't be done at the time to let Druid release go forward quickly, so the change was reverted. Later, original problem got fixed and now it is [rolling eyes] safe to bring the change back.

[nishantmonu51]

LGTM, 👍

[clintropolis]

🤘

[himanshug]

thanks @clintropolis , @nishantmonu51

I also tested it manually to see that stuff in sys.segments and sys.tasks table continues to work when coordinator/overlord leadership changes.

[suneet-s]

@himanshug How should we write an integration test for this so that we do not need to test this manually with each release? Can you describe the steps you did. Is there something missing in the integration tests framework that prevents us from writing an integration test for this?

[himanshug]

@suneet-s for the specific change here, it does get covered by integration tests but scenarios like one of the leader overlord/coordinator node crash while some operation was happening, don't get tested in our build environments as all of them work with one overlord and coordinator nodes . Even in a real more complex env we would need to simulate node crashes to handle such scenarios to simulate different possible leader/node failure models.
does that answer your question ?