address comments

Signed-off-by: Abhishek Kumar <[email protected]>
apache · Nov 5, 2024 · 7d43ce3 · 7d43ce3
1 parent 779d521
commit 7d43ce3
Show file tree

Hide file tree

Showing 16 changed files with 59 additions and 46 deletions.
diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties
@@ -434,8 +434,9 @@ iscsi.session.cleanup.enabled=false
 # Implicit host tags managed by agent.properties
 # host.tags=
 
-# Timeout(in seconds) for SSL handshake when agent connects to server
+# Timeout(in seconds) for SSL handshake when agent connects to server. When no value is set then default value of 30s
+# will be used
 #ssl.handshake.timeout=
 
-# Wait(in seconds) during agent reconnections
+# Wait(in seconds) during agent reconnections. When no value is set then default value of 5s will be used
 #backoff.seconds=
diff --git a/agent/src/main/java/com/cloud/agent/Agent.java b/agent/src/main/java/com/cloud/agent/Agent.java
@@ -526,7 +526,7 @@ public void sendStartup(final Link link) {
 
     protected String retrieveHostname() {
         if (logger.isTraceEnabled()) {
-            logger.trace(" Retrieving hostname " + serverResource.getClass().getSimpleName());
+            logger.trace("Retrieving hostname with resource={}", serverResource.getClass().getSimpleName());
         }
         final String result = Script.runSimpleBashScript(Script.getExecutableAbsolutePath("hostname"), 500);
         if (StringUtils.isNotBlank(result)) {

diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
@@ -179,7 +179,7 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
 
     protected ExecutorService _executor;
     protected ThreadPoolExecutor _connectExecutor;
-    protected ThreadPoolExecutor _directAgentExecutor;
+    protected ScheduledExecutorService _directAgentExecutor;
     protected ScheduledExecutorService _cronJobExecutor;
     protected ScheduledExecutorService _monitorExecutor;
 
@@ -243,12 +243,13 @@ public boolean configure(final String name, final Map<String, Object> params) th
                 caService, RemoteAgentSslHandshakeTimeout.value());
         logger.info("Listening on {} with {} workers.", Port.value(), Workers.value());
 
+        final int directAgentPoolSize = DirectAgentPoolSize.value();
         // executes all agent commands other than cron and ping
-        _directAgentExecutor = new ThreadPoolExecutor(Math.max(agentTaskThreads/10, 1), DirectAgentPoolSize.value(), 120L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), new NamedThreadFactory("DirectAgent"));
+        _directAgentExecutor = new ScheduledThreadPoolExecutor(directAgentPoolSize, new NamedThreadFactory("DirectAgent"));
         // executes cron and ping agent commands
-        _cronJobExecutor = new ScheduledThreadPoolExecutor(DirectAgentPoolSize.value(), new NamedThreadFactory("DirectAgentCronJob"));
-        logger.debug("Created DirectAgentAttache pool with size: {}.", DirectAgentPoolSize.value());
-        _directAgentThreadCap = Math.round(DirectAgentPoolSize.value() * DirectAgentThreadCap.value()) + 1; // add 1 to always make the value > 0
+        _cronJobExecutor = new ScheduledThreadPoolExecutor(directAgentPoolSize, new NamedThreadFactory("DirectAgentCronJob"));
+        logger.debug("Created DirectAgentAttache pool with size: {}.", directAgentPoolSize);
+        _directAgentThreadCap = Math.round(directAgentPoolSize * DirectAgentThreadCap.value()) + 1; // add 1 to always make the value > 0
 
         _monitorExecutor = new ScheduledThreadPoolExecutor(1, new NamedThreadFactory("AgentMonitor"));
 
@@ -1642,7 +1643,7 @@ public void pullAgentOutMaintenance(final long hostId) {
         }
     }
 
-    public ThreadPoolExecutor getDirectAgentPool() {
+    public ScheduledExecutorService getDirectAgentPool() {
         return _directAgentExecutor;
     }
 

diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -3803,7 +3803,6 @@ public int getTimeout() {
     public boolean processCommands(final long agentId, final long seq, final Command[] cmds) {
         boolean processed = false;
         for (final Command cmd : cmds) {
-            // FIXME: PingRoutingCommand handler is DB & CPU hotspot
             if (cmd instanceof PingRoutingCommand) {
                 final PingRoutingCommand ping = (PingRoutingCommand)cmd;
                 if (ping.getHostVmStateReport() != null) {

diff --git a/engine/schema/src/main/java/com/cloud/dc/dao/ClusterDao.java b/engine/schema/src/main/java/com/cloud/dc/dao/ClusterDao.java
@@ -59,6 +59,4 @@ public interface ClusterDao extends GenericDao<ClusterVO, Long> {
     List<CPU.CPUArch> getClustersArchsByZone(long zoneId);
 
     List<ClusterVO> listClustersByArchAndZoneId(long zoneId, CPU.CPUArch arch);
-
-    List<Long> listAllIds();
 }
diff --git a/engine/schema/src/main/java/com/cloud/dc/dao/ClusterDaoImpl.java b/engine/schema/src/main/java/com/cloud/dc/dao/ClusterDaoImpl.java
@@ -171,12 +171,10 @@ public List<HypervisorType> getAvailableHypervisorInZone(Long zoneId) {
             sc.setParameters("zoneId", zoneId);
         }
         List<ClusterVO> clusters = listBy(sc);
-        List<HypervisorType> distinctHypervisors = new ArrayList<>(4);
-        for (ClusterVO cluster : clusters) {
-            distinctHypervisors.add(cluster.getHypervisorType());
-        }
-
-        return distinctHypervisors;
+        return clusters.stream()
+                .map(ClusterVO::getHypervisorType)
+                .distinct()
+                .collect(Collectors.toList());
     }
 
     @Override
@@ -346,12 +344,4 @@ public List<ClusterVO> listClustersByArchAndZoneId(long zoneId, CPU.CPUArch arch
         sc.setParameters("arch", arch);
         return listBy(sc);
     }
-
-    @Override
-    public List<Long> listAllIds() {
-        GenericSearchBuilder<ClusterVO, Long> sb = createSearchBuilder(Long.class);
-        sb.selectFields(sb.entity().getId());
-        sb.done();
-        return customSearch(sb.create(), null);
-    }
 }
diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java
@@ -192,8 +192,6 @@ List<Long> findHostIdsByZoneClusterResourceStateTypeAndHypervisorType(final Long
             final List<ResourceState> resourceStates, final List<Type> types,
             final List<Hypervisor.HypervisorType> hypervisorTypes);
 
-    List<Long> listAllIds();
-
     List<HypervisorType> listDistinctHypervisorTypes(final Long zoneId);
 
     List<HostVO> listByIds(final List<Long> ids);

diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
@@ -1743,11 +1743,6 @@ public List<Long> findHostIdsByZoneClusterResourceStateTypeAndHypervisorType(fin
         return customSearch(sc, null);
     }
 
-    @Override
-    public List<Long> listAllIds() {
-        return listIdsBy(null, null, null, null, null, null, null);
-    }
-
     @Override
     public List<HypervisorType> listDistinctHypervisorTypes(final Long zoneId) {
         GenericSearchBuilder<HostVO, HypervisorType> sb = createSearchBuilder(HypervisorType.class);

diff --git a/.../schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java b/.../schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java
@@ -154,6 +154,4 @@ Pair<List<Long>, Integer> searchForIdsAndCount(Long storagePoolId, String storag
             String keyword, Filter searchFilter);
 
     List<StoragePoolVO> listByIds(List<Long> ids);
-
-    List<Long> listAllIds();
 }
diff --git a/...ema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java b/...ema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
@@ -800,12 +800,4 @@ private SearchCriteria<StoragePoolVO> createStoragePoolSearchCriteria(Long stora
         sc.setParameters("parent", 0);
         return sc;
     }
-
-    @Override
-    public List<Long> listAllIds() {
-        GenericSearchBuilder<StoragePoolVO, Long> sb = createSearchBuilder(Long.class);
-        sb.selectFields(sb.entity().getId());
-        sb.done();
-        return customSearch(sb.create(), null);
-    }
 }
diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java
@@ -148,6 +148,11 @@ public interface GenericDao<T, ID extends Serializable> {
      */
     List<T> listAll(Filter filter);
 
+    /**
+     * Look IDs for all active rows.
+     */
+    List<ID> listAllIds();
+
     /**
      * Search for the entity beans
      * @param sc

diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java
@@ -1214,6 +1214,35 @@ public List<T> listAll(final Filter filter) {
         return executeList(sql.toString());
     }
 
+    private Object getIdObject() {
+        T entity = (T)_searchEnhancer.create();
+        try {
+            Method m = _entityBeanType.getMethod("getId");
+            return m.invoke(entity);
+        } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException ignored) {
+            logger.warn(String.format("Unable to get ID object for entity: %s", _entityBeanType.getSimpleName()));
+        }
+        return null;
+    }
+
+    @Override
+    public List<ID> listAllIds() {
+        Object idObj = getIdObject();
+        if (idObj == null) {
+            return Collections.emptyList();
+        }
+        Class<ID> clazz = (Class<ID>)idObj.getClass();
+        GenericSearchBuilder<T, ID> sb = createSearchBuilder(clazz);
+        try {
+            Method m = sb.entity().getClass().getMethod("getId");
+            sb.selectFields(m.invoke(sb.entity()));
+        } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException ignored) {
+            return Collections.emptyList();
+        }
+        sb.done();
+        return customSearch(sb.create(), null);
+    }
+
     @Override
     public boolean expunge(final ID id) {
         final TransactionLegacy txn = TransactionLegacy.currentTxn();

diff --git a/plugins/hypervisors/simulator/src/main/java/com/cloud/resource/AgentRoutingResource.java b/plugins/hypervisors/simulator/src/main/java/com/cloud/resource/AgentRoutingResource.java
@@ -109,7 +109,6 @@ public Type getType() {
     public PingCommand getCurrentStatus(long id) {
         TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.SIMULATOR_DB);
         try {
-            //MockConfigurationVO config = _simMgr.getMockConfigurationDao().findByNameBottomUP(agentHost.getDataCenterId(), agentHost.getPodId(), agentHost.getClusterId(), agentHost.getId(), "PingCommand");
             MockConfigurationVO config = null;
             if (config != null) {
                 Map<String, String> configParameters = config.getParameters();
@@ -123,7 +122,6 @@ public PingCommand getCurrentStatus(long id) {
                 }
             }
 
-            //config = _simMgr.getMockConfigurationDao().findByNameBottomUP(agentHost.getDataCenterId(), agentHost.getPodId(), agentHost.getClusterId(), agentHost.getId(), "PingRoutingWithNwGroupsCommand");
             config = null;
             if (config != null) {
                 String message = config.getJsonResponse();

diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
@@ -1558,6 +1558,9 @@ protected void cleanupConnectedHostConnectionForFailedStorage(DataStore primaryS
     @Override
     public void connectHostsToPool(DataStore primaryStore, List<Long> hostIds, Scope scope,
               boolean handleExceptionsPartially, boolean errorOnNoUpHost) throws CloudRuntimeException {
+        if (CollectionUtils.isEmpty(hostIds)) {
+            return;
+        }
         CopyOnWriteArrayList<Long> poolHostIds = new CopyOnWriteArrayList<>();
         ExecutorService executorService = Executors.newFixedThreadPool(Math.max(1, Math.min(hostIds.size(),
                 StoragePoolHostConnectWorkers.value())));

diff --git a/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java b/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java
@@ -594,7 +594,7 @@ public String getConfigComponentName() {
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {RoleService.EnableDynamicApiChecker, RoleService.DynamicApiCheckerCachePeriod};
+        return new ConfigKey<?>[] {EnableDynamicApiChecker, DynamicApiCheckerCachePeriod};
     }
 
     @Override

diff --git a/server/src/test/java/com/cloud/user/MockUsageEventDao.java b/server/src/test/java/com/cloud/user/MockUsageEventDao.java
@@ -27,6 +27,7 @@
 
 import javax.naming.ConfigurationException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
@@ -142,6 +143,11 @@ public List<UsageEventVO> listAll(Filter filter) {
         return null;
     }
 
+    @Override
+    public List<Long> listAllIds() {
+        return Collections.emptyList();
+    }
+
     @Override
     public List<UsageEventVO> search(SearchCriteria<UsageEventVO> sc,
             Filter filter) {