This is an automated email from the ASF dual-hosted git repository.
amagyar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new bc4d5486b KNOX-2978 - Race condition between Service Discovery and
Polling Config Analyzer (#811)
bc4d5486b is described below
commit bc4d5486bb1b60728a1c6376336a13c627c5aa5b
Author: Attila Magyar <[email protected]>
AuthorDate: Fri Oct 27 11:21:05 2023 +0200
KNOX-2978 - Race condition between Service Discovery and Polling Config
Analyzer (#811)
---
.../discovery/cm/ClouderaManagerServiceDiscovery.java | 4 +---
.../cm/ClouderaManagerServiceDiscoveryRepository.java | 9 ++-------
.../cm/ClouderaManagerServiceDiscoveryRepositoryTest.java | 12 +++---------
3 files changed, 6 insertions(+), 19 deletions(-)
diff --git
a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java
b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java
index b74c2a26b..8d1227a56 100644
---
a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java
+++
b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java
@@ -253,8 +253,6 @@ public class ClouderaManagerServiceDiscovery implements
ServiceDiscovery, Cluste
log.discoveringCluster(clusterName);
- repository.registerCluster(client.getConfig());
-
Set<ServiceModel> serviceModels = new HashSet<>();
List<ApiService> serviceList = getClusterServices(client.getConfig(),
servicesResourceApi);
@@ -352,7 +350,7 @@ public class ClouderaManagerServiceDiscovery implements
ServiceDiscovery, Cluste
private List<ApiService> getClusterServices(ServiceDiscoveryConfig
serviceDiscoveryConfig, ServicesResourceApi servicesResourceApi) throws
ApiException {
log.lookupClusterServicesFromRepository();
List<ApiService> services = repository.getServices(serviceDiscoveryConfig);
- if (services == null || services.isEmpty()) {
+ if (services.isEmpty()) {
try {
log.lookupClusterServicesFromCM();
final ApiServiceList serviceList =
servicesResourceApi.readServices(serviceDiscoveryConfig.getCluster(),
VIEW_SUMMARY);
diff --git
a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java
b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java
index 5329cd8ce..115717f69 100644
---
a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java
+++
b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java
@@ -59,23 +59,18 @@ class ClouderaManagerServiceDiscoveryRepository {
repository.clear();
}
- void registerCluster(ServiceDiscoveryConfig serviceDiscoveryConfig) {
- repository.putIfAbsent(RepositoryKey.of(serviceDiscoveryConfig),
Caffeine.newBuilder().expireAfterWrite(Duration.ofSeconds(cacheEntryTTL)).build());
- }
-
void addService(ServiceDiscoveryConfig serviceDiscoveryConfig, ApiService
service) {
getClusterServices(serviceDiscoveryConfig).put(service, new
ServiceDetails());
}
List<ApiService> getServices(ServiceDiscoveryConfig serviceDiscoveryConfig) {
final Cache<ApiService, ServiceDetails> clusterServices =
getClusterServices(serviceDiscoveryConfig);
- return clusterServices == null ? null
- : clusterServices.asMap().entrySet().stream().filter(entry ->
entry.getValue() != null).map(entry -> entry.getKey())
+ return clusterServices.asMap().entrySet().stream().filter(entry ->
entry.getValue() != null).map(entry -> entry.getKey())
.collect(Collectors.toList());
}
private Cache<ApiService, ServiceDetails>
getClusterServices(ServiceDiscoveryConfig serviceDiscoveryConfig) {
- return repository.get(RepositoryKey.of(serviceDiscoveryConfig));
+ return
repository.computeIfAbsent(RepositoryKey.of(serviceDiscoveryConfig), (k) ->
Caffeine.newBuilder().expireAfterWrite(Duration.ofSeconds(cacheEntryTTL)).build());
}
void addServiceConfig(ServiceDiscoveryConfig serviceDiscoveryConfig,
ApiService service, ApiServiceConfig serviceConfig) {
diff --git
a/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java
b/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java
index b20f93d6f..d65408272 100644
---
a/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java
+++
b/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java
@@ -56,14 +56,13 @@ public class ClouderaManagerServiceDiscoveryRepositoryTest {
@Before
public void setUp() {
repository.clear();
- assertNull(repository.getServices(serviceDiscoveryConfig));
repository.setCacheEntryTTL(GatewayConfig.DEFAULT_CM_SERVICE_DISCOVERY_CACHE_ENTRY_TTL);
+ assertTrue(repository.getServices(serviceDiscoveryConfig).isEmpty());
}
@Test
public void testRegisterCluster() throws Exception {
- assertNull(repository.getServices(serviceDiscoveryConfig));
- repository.registerCluster(serviceDiscoveryConfig);
+ assertTrue(repository.getServices(serviceDiscoveryConfig).isEmpty());
final List<ApiService> services =
repository.getServices(serviceDiscoveryConfig);
assertNotNull(services);
assertTrue(services.isEmpty());
@@ -72,7 +71,6 @@ public class ClouderaManagerServiceDiscoveryRepositoryTest {
@Test
public void testAddService() throws Exception {
final String serviceName = "HDFS-1";
- repository.registerCluster(serviceDiscoveryConfig);
assertFalse(containsService(serviceName));
final ApiService service = EasyMock.createNiceMock(ApiService.class);
EasyMock.expect(service.getName()).andReturn(serviceName).anyTimes();
@@ -86,7 +84,6 @@ public class ClouderaManagerServiceDiscoveryRepositoryTest {
final String serviceName = "HDFS-1";
final String serviceConfigName = "myServiceConfigName";
final String serviceConfigValue = "myServiceConfigValue";
- repository.registerCluster(serviceDiscoveryConfig);
final ApiService service = EasyMock.createNiceMock(ApiService.class);
EasyMock.expect(service.getName()).andReturn(serviceName).anyTimes();
final ApiConfig serviceConfig = EasyMock.createNiceMock(ApiConfig.class);
@@ -105,7 +102,6 @@ public class ClouderaManagerServiceDiscoveryRepositoryTest {
public void testAddRole() throws Exception {
final String serviceName = "HDFS-1";
final String roleName = "NAMENODE-1";
- repository.registerCluster(serviceDiscoveryConfig);
final ApiService service = EasyMock.createNiceMock(ApiService.class);
EasyMock.expect(service.getName()).andReturn(serviceName).anyTimes();
final ApiRole role = EasyMock.createNiceMock(ApiRole.class);
@@ -121,7 +117,6 @@ public class ClouderaManagerServiceDiscoveryRepositoryTest {
@Test
public void testAddNullRoles() throws Exception {
- repository.registerCluster(serviceDiscoveryConfig);
final ApiService service = EasyMock.createNiceMock(ApiService.class);
EasyMock.expect(service.getName()).andReturn(ClouderaManagerServiceDiscovery.CORE_SETTINGS_TYPE).anyTimes();
EasyMock.replay(service);
@@ -132,7 +127,6 @@ public class ClouderaManagerServiceDiscoveryRepositoryTest {
@Test
public void testAddNullRoleItems() throws Exception {
- repository.registerCluster(serviceDiscoveryConfig);
final ApiService service = EasyMock.createNiceMock(ApiService.class);
EasyMock.expect(service.getName()).andReturn(ClouderaManagerServiceDiscovery.CORE_SETTINGS_TYPE).anyTimes();
final ApiRoleList roles = EasyMock.createNiceMock(ApiRoleList.class);
@@ -150,7 +144,6 @@ public class ClouderaManagerServiceDiscoveryRepositoryTest {
final String roleConfigName = "myRoleConfig";
final String roleConfigValue = "myRoleConfigValue";
- repository.registerCluster(serviceDiscoveryConfig);
final ApiService service = EasyMock.createNiceMock(ApiService.class);
EasyMock.expect(service.getName()).andReturn(serviceName).anyTimes();
final ApiRole role = EasyMock.createNiceMock(ApiRole.class);
@@ -174,6 +167,7 @@ public class ClouderaManagerServiceDiscoveryRepositoryTest {
public void testCacheAutoEviction() throws Exception {
final long entryTTL = 1;
repository.setCacheEntryTTL(entryTTL);
+ repository.clear();
testAddService();
TimeUnit.SECONDS.sleep(entryTTL + 1);
assertFalse(containsService("HDFS-1"));
