Fix ZkNoNodeException when zookeeper server is empty. #901

ujjboy · 2019-04-25T04:04:52Z

Ⅰ. Describe what this PR did

fixes Parent node is not automatically created in the Zookeeper discovery. #900
add test case.

Ⅱ. Does this pull request fix one issue?

Fixes #900

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-io · 2019-04-25T04:08:26Z

Codecov Report

Merging #901 into develop will decrease coverage by 0.37%.
The diff coverage is 0%.

@@              Coverage Diff              @@
##             develop     #901      +/-   ##
=============================================
- Coverage       38.7%   38.33%   -0.38%     
- Complexity      1026     1028       +2     
=============================================
  Files            220      222       +2     
  Lines           8492     8621     +129     
  Branches        1023     1044      +21     
=============================================
+ Hits            3287     3305      +18     
- Misses          4814     4926     +112     
+ Partials         391      390       -1

Impacted Files	Coverage Δ	Complexity Δ
...very/registry/zk/ZookeeperRegisterServiceImpl.java	`0% <0%> (ø)`	`0 <0> (?)`
...scovery/registry/zk/ZookeeperRegistryProvider.java	`0% <0%> (ø)`	`0% <0%> (?)`
...eata/server/store/FileTransactionStoreManager.java	`53.5% <0%> (+0.5%)`	`15% <0%> (ø)`	⬇️
...o/seata/server/coordinator/DefaultCoordinator.java	`53.57% <0%> (+1.42%)`	`16% <0%> (ø)`	⬇️
...eata/core/rpc/netty/AbstractRpcRemotingClient.java	`20.57% <0%> (+3.34%)`	`7% <0%> (ø)`	⬇️
...ain/java/io/seata/core/rpc/netty/ShutdownHook.java	`81.25% <0%> (+25%)`	`8% <0%> (+2%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b108de5...021e0a8. Read the comment docs.

discovery/seata-discovery-zk/pom.xml

...covery-zk/src/test/java/io/seata/discovery/registry/zk/ZookeeperRegisterServiceImplTest.java

...discovery-zk/src/test/java/io/seata/discovery/registry/zk/ZookeeperRegistryProviderTest.java

xingfudeshi

thanks.i left some comments.

ujjboy · 2019-04-25T05:29:26Z

@xingfudeshi done~

xingfudeshi · 2019-04-25T05:31:59Z

...discovery-zk/src/test/java/io/seata/discovery/registry/zk/ZookeeperRegistryProviderTest.java

+        ZookeeperRegistryProvider provider = new ZookeeperRegistryProvider();
+        RegistryService registryService = provider.provide();
+        Assert.assertTrue(registryService instanceof ZookeeperRegisterServiceImpl);
+        Assert.assertEquals(registryService, provider.provide());


this line can be removed.

This line is for testing the return singleton ZookeeperRegisterServiceImpl .

it's not necessary.the code
Assert.assertEquals(registryService, provider.provide()); will always pass even the registryService is null,because the registryService was returned by calling provider.provide()
RegistryService registryService = provider.provide();

xingfudeshi · 2019-04-25T05:34:50Z

...discovery-zk/src/test/java/io/seata/discovery/registry/zk/ZookeeperRegistryProviderTest.java

+    @Test
+    public void provide() {
+        ZookeeperRegistryProvider provider = new ZookeeperRegistryProvider();
+        RegistryService registryService = provider.provide();


and this line can be removed too.
the Assert.assertTrue(registryService instanceof ZookeeperRegisterServiceImpl);
can be replaced with the following code:
Assert.assertTrue(provider.provide() instanceof ZookeeperRegisterServiceImpl);

This line is for testing the return singleton ZookeeperRegisterServiceImpl .

xingfudeshi

LGTM.

zhangthen

+1

Fix ZkNoNodeException when zookeeper server is empty.

e425a77