g3rg0 commented on code in PR #6677:
URL: https://github.com/apache/hive/pull/6677#discussion_r3728801346
##########
llap-client/src/test/org/apache/hadoop/hive/llap/registry/impl/TestLlapZookeeperRegistryImpl.java:
##########
@@ -99,6 +108,66 @@ public void testRegister() throws Exception {
parseInt(attributes.get(LlapRegistryService.LLAP_DAEMON_NUM_ENABLED_EXECUTORS)));
}
+ @Test
+ public void testRetryOnInvalidACLException() throws Exception {
+ // Given
+ LlapZookeeperRegistryImpl underTest =
+ new LlapZookeeperRegistryImpl("ClientRegistryRetryTest", hiveConf);
+
+ ACLProvider aclProvider = Mockito.mock(ACLProvider.class);
+ ACL allowAll = new ACL(ZooDefs.Perms.ALL, ZooDefs.Ids.ANYONE_ID_UNSAFE);
+ Mockito.when(aclProvider.getAclForPath(Mockito.any())).
+ thenReturn(Collections.emptyList()). // causes InvalidACLException
+ thenReturn(Collections.singletonList(allowAll)); // allow all
+
+ CuratorFramework curatorFrameworkWithAclProvider = CuratorFrameworkFactory.
+ builder().
+ connectString(server.getConnectString()).
+ sessionTimeoutMs(10000).
+ retryPolicy(new RetryOneTime(1000)).
+ aclProvider(aclProvider).
+ build();
+
+ trySetMock(underTest, "zooKeeperClient", curatorFrameworkWithAclProvider);
+ underTest.start();
+
+ // When
+ ServiceInstanceSet<LlapServiceInstance> serviceInstanceSet =
+ underTest.getInstances("LLAP", 10000);
+
+ // Then
+ Collection<LlapServiceInstance> llaps = serviceInstanceSet.getAll();
+ assertEquals(0, llaps.size());
+ Mockito.verify(aclProvider,
Mockito.atLeast(4)).getAclForPath(Mockito.any());
+ }
+
+ @Test
+ public void testClusterNotReadyExceptionIsThrownWhenZkNodeNotExists() throws
Exception {
Review Comment:
Done. We addressed this both ways:
1. The existing test (timeout=0) has been renamed to
testClusterNotReadyExceptionOnImmediateTimeoutWithSecureAcl to accurately
reflect that it tests the immediate-failure path, not retry exhaustion.
2. A new test testClusterNotReadyExceptionAfterRetriesWithSecureAcl has
been added with a small positive timeout (100ms). It asserts that: (a)
ClusterNotReadyException is thrown after the deadline, (b) elapsed time is at
least 16ms (the initial retry sleep), confirming retries occurred, and (c)
getAclForPath was called at least twice. This keeps the test deterministic
while exercising the retry-until-deadline behavior.
##########
llap-client/src/test/org/apache/hadoop/hive/llap/registry/impl/TestLlapZookeeperRegistryImpl.java:
##########
@@ -99,6 +108,66 @@ public void testRegister() throws Exception {
parseInt(attributes.get(LlapRegistryService.LLAP_DAEMON_NUM_ENABLED_EXECUTORS)));
}
+ @Test
+ public void testRetryOnInvalidACLException() throws Exception {
+ // Given
+ LlapZookeeperRegistryImpl underTest =
+ new LlapZookeeperRegistryImpl("ClientRegistryRetryTest", hiveConf);
+
+ ACLProvider aclProvider = Mockito.mock(ACLProvider.class);
+ ACL allowAll = new ACL(ZooDefs.Perms.ALL, ZooDefs.Ids.ANYONE_ID_UNSAFE);
+ Mockito.when(aclProvider.getAclForPath(Mockito.any())).
+ thenReturn(Collections.emptyList()). // causes InvalidACLException
+ thenReturn(Collections.singletonList(allowAll)); // allow all
+
+ CuratorFramework curatorFrameworkWithAclProvider = CuratorFrameworkFactory.
+ builder().
+ connectString(server.getConnectString()).
+ sessionTimeoutMs(10000).
+ retryPolicy(new RetryOneTime(1000)).
+ aclProvider(aclProvider).
+ build();
+
+ trySetMock(underTest, "zooKeeperClient", curatorFrameworkWithAclProvider);
+ underTest.start();
+
+ // When
+ ServiceInstanceSet<LlapServiceInstance> serviceInstanceSet =
+ underTest.getInstances("LLAP", 10000);
+
+ // Then
+ Collection<LlapServiceInstance> llaps = serviceInstanceSet.getAll();
+ assertEquals(0, llaps.size());
+ Mockito.verify(aclProvider,
Mockito.atLeast(4)).getAclForPath(Mockito.any());
+ }
+
+ @Test
+ public void testClusterNotReadyExceptionIsThrownWhenZkNodeNotExists() throws
Exception {
+ // Given
+ LlapZookeeperRegistryImpl underTest =
+ new LlapZookeeperRegistryImpl("ClientRegistryClusterNotReadyTest",
hiveConf);
+
+ ACLProvider aclProvider = Mockito.mock(ACLProvider.class);
+ List<ACL> secureAcls = new ArrayList<>();
+ secureAcls.addAll(ZooDefs.Ids.READ_ACL_UNSAFE); // Read all to the world
+ secureAcls.addAll(ZooDefs.Ids.CREATOR_ALL_ACL); //
Create/Delete/Write/Admin to creator
+
Mockito.when(aclProvider.getAclForPath(Mockito.any())).thenReturn(secureAcls);
+ CuratorFramework curatorFrameworkWithAclProvider = CuratorFrameworkFactory.
+ builder().
+ connectString(server.getConnectString()).
+ sessionTimeoutMs(10000).
+ retryPolicy(new RetryOneTime(1000)).
+ aclProvider(aclProvider).
+ build();
+
+ trySetMock(underTest, "zooKeeperClient", curatorFrameworkWithAclProvider);
+ underTest.start();
+
+ // When - Then
+ assertThrows(ClusterNotReadyException.class,
+ () -> underTest.getInstances("LLAP", 0));
Review Comment:
Done. See the previous comment.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
