tdunning commented on issue #1048: ZOOKEEPER-3188: Improve resilience to network URL: https://github.com/apache/zookeeper/pull/1048#issuecomment-539555135 On Tue, Oct 8, 2019 at 6:49 AM Mate Szalay-Beko <[email protected]> wrote: > *Thanks for all of you for the comments and review so far!* > Nice work to you as well. > ... An important change: due to the way how we represent the server > configs during dynamic reconfig, I had to change the separator character > from comma to pipe. > This is the predictable penalty of an ad hoc syntax that isn't consistent everywhere. Good catch. > > - > - ...I actually saw during the tests, that simply adding new > addresses for servers with incremental re-config does not cause a new > leader election (although I haven't created an automated test explicitly > forcing this). I am not sure if it is worth to add a new test for this, as > dynamic reconfigs are relatively rare events during production use I guess. > > This feels nice, but it is actually a little worrisome in that it seems to indicate that the multi connect stuff is more clever than I would expect. > *I try to summarize the open discussions / tasks* to see how much work is > required to close this issue: > > - we should add the new config format to the documentation > - we should extend the dynamic reconfig documentation with the > comments above > - we should test (for now I propose only to try out manually) and > document: > - the impact on quorum TLS / kerberos feature > - the impact on rolling upgrades > - and of course we need to resolve any code related conversation what > would be still open (e.g. on parallel streams with @eolivelli > <https://github.com/eolivelli> ) > > I think that this is a good summary. > > > *I think if the tasks above are concluded, then it is safe to merge the > PR.* What do you think? > I am only half informed on this, but your summary seems persuasive. > The automated integration tests would be nice to have definitely. These > test cases I have in mind: > > - Automated integration test for rolling upgrades (e.g. from 3.5.6 to > master) without changing the config > - Automated integration test for rolling upgrades (e.g. from 3.5.6 to > master) with changing the config to use multiple addresses during the > upgrade > - Automated integration test for simulating the case of network > failure during the use of multiple addresses > - hint: use docker with multiple network interfaces (see: > https://github.com/symat/zookeeper-docker-test ) > - I am doing this test manually after each of my commits > > *I would cover these integration tests in follow-up jira ticket(s?), as > these seems to be not trivial to automatize.* What do you think? > Without an advanced test framework that can create and control containers, I think that these kinds of tests are difficult to create and maintain. To me, the key question is how much value would these automated tests actually provide. I see very high value in a documented procedure and outcome for a manual version of these tests, but I am not sure I see a high value for even repeating these manual tests again. How likely is it that subsequent changes will cause these tests to fail? Is that probability high enough to justify the considerable cost of automation?
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
