You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/10/27 19:35:02 UTC

[GitHub] [druid] himanshug opened a new pull request #10537: remove ServerDiscoverySelector i.e. hardcoded zk dependency from DruidLeaderClient

himanshug opened a new pull request #10537:
URL: https://github.com/apache/druid/pull/10537


   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.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] himanshug commented on pull request #10537: remove ServerDiscoverySelector i.e. hardcoded zk dependency from DruidLeaderClient

Posted by GitBox <gi...@apache.org>.
himanshug commented on pull request #10537:
URL: https://github.com/apache/druid/pull/10537#issuecomment-721315754


   @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 node failure models.
   does that answer your question ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] himanshug merged pull request #10537: remove ServerDiscoverySelector i.e. hardcoded zk dependency from DruidLeaderClient

Posted by GitBox <gi...@apache.org>.
himanshug merged pull request #10537:
URL: https://github.com/apache/druid/pull/10537


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] suneet-s commented on pull request #10537: remove ServerDiscoverySelector i.e. hardcoded zk dependency from DruidLeaderClient

Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #10537:
URL: https://github.com/apache/druid/pull/10537#issuecomment-720873591


   @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?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] himanshug edited a comment on pull request #10537: remove ServerDiscoverySelector i.e. hardcoded zk dependency from DruidLeaderClient

Posted by GitBox <gi...@apache.org>.
himanshug edited a comment on pull request #10537:
URL: https://github.com/apache/druid/pull/10537#issuecomment-721315754


   @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 ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] himanshug commented on pull request #10537: remove ServerDiscoverySelector i.e. hardcoded zk dependency from DruidLeaderClient

Posted by GitBox <gi...@apache.org>.
himanshug commented on pull request #10537:
URL: https://github.com/apache/druid/pull/10537#issuecomment-718106802


   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.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org