[CSTACKEX-204] Resolved Review Comments

Author: Gupta, Surya

engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java

@@ -96,6 +96,8 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol
     protected static final String SELECT_HYPERTYPE_FROM_CLUSTER_VOLUME = "SELECT c.hypervisor_type from volumes v, storage_pool s, cluster c where v.pool_id = s.id and s.cluster_id = c.id and v.id = ?";
     protected static final String SELECT_HYPERTYPE_FROM_ZONE_VOLUME = "SELECT s.hypervisor from volumes v, storage_pool s where v.pool_id = s.id and v.id = ?";
     protected static final String SELECT_POOLSCOPE = "SELECT s.scope from storage_pool s, volumes v where s.id = v.pool_id and v.id = ?";
+    // Looks for a VM whose root disk is on this pool and whose data disk is on another pool.
+    // LIMIT 1 stops after the first match so we do not load all volumes.
     private static final String HAS_MULTI_PRIMARY_STORAGE_POOL_VM =
             "SELECT 1 FROM volumes root INNER JOIN volumes data ON data.instance_id = root.instance_id "
                     + "WHERE root.pool_id = ? AND root.volume_type = 'ROOT' AND root.instance_id IS NOT NULL "

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java

@@ -84,7 +84,7 @@ public class OntapAsupManager extends ManagerBase {
     private static final int ASUP_LOCK_TIMEOUT_SECONDS = 5;
 
     /**
-     * Fixed wakeup interval (ms) for {@link OntapAsupPollTask} (2 hours). The task wakes on
+     * Fixed wakeup interval (ms) for {@link OntapAsupPollTask} (1 hour). The task wakes on
      * this cadence and checks whether the live configured push interval
      * ({@link OntapConfigurationManager#AsupIntervalSeconds}) has elapsed. UI edits of that
      * interval are applied immediately via the configuration-edit event; this delay is only

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java

@@ -166,7 +166,7 @@ public class OntapStorageConstants {
     public static final String ASUP_ENABLED_CONFIG_KEY = "ontap.asup.enabled";
     public static final String ASUP_ENABLED_DEFAULT = "true";
     public static final String ASUP_INTERVAL_CONFIG_KEY = "ontap.asup.interval";
-    public static final int ASUP_MIN_INTERVAL_SECONDS = 60; // 1 minute (demo; production is 10800)
+    public static final int ASUP_MIN_INTERVAL_SECONDS = 10800; // 3 hours
     public static final int ASUP_MAX_INTERVAL_SECONDS = 86400; // 24 hours
     public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 43200; // 12 hours (twice a day)
 
@@ -176,7 +176,7 @@ public class OntapStorageConstants {
      * window. Config edits are applied immediately via the configuration-edit event; this
      * delay is only the background check, not the ASUP push interval.
      */
-    public static final int ASUP_POLL_CHECK_INTERVAL_SECONDS = 10; // 10 seconds (demo; production is 7200)
+    public static final int ASUP_POLL_CHECK_INTERVAL_SECONDS = 3600; // 1 hour
 
     public static final String ASUP_ENABLED_DESCRIPTION =
             "Set to true to enable telemetry reporting from the CloudStack ONTAP plugin, or false to disable it. "

plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java

@@ -500,7 +500,7 @@ void validateAsupInterval_acceptsMinMaxAndDefault() {
 
     @Test
     void validateAsupInterval_rejectsOutOfRangeAndNonInteger() {
-        assertThrows(InvalidParameterValueException.class, () -> OntapConfigurationManager.AsupIntervalSeconds.validateValue("59"));
+        assertThrows(InvalidParameterValueException.class, () -> OntapConfigurationManager.AsupIntervalSeconds.validateValue("10799"));
         assertThrows(InvalidParameterValueException.class, () -> OntapConfigurationManager.AsupIntervalSeconds.validateValue("86401"));
         assertThrows(InvalidParameterValueException.class, () -> OntapConfigurationManager.AsupIntervalSeconds.validateValue("0"));
         assertThrows(InvalidParameterValueException.class, () -> OntapConfigurationManager.AsupIntervalSeconds.validateValue("abc"));
@@ -514,7 +514,7 @@ void getAsupIntervalSeconds_fallsBackOutsideRange() {
         assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS,
                 asupManager.getAsupIntervalSeconds(0));
         assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS,
-                asupManager.getAsupIntervalSeconds(59));
+                asupManager.getAsupIntervalSeconds(10799));
         assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS,
                 asupManager.getAsupIntervalSeconds(86401));
         assertEquals(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS,