Performance improvements of AgentPropertiesFileHandler

Author: mprokopchuk

agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java

@@ -23,6 +23,7 @@
 import java.util.Map;
 import java.util.Properties;
 
+import com.cloud.agent.properties.AgentPropertiesFileHandler;
 import org.apache.commons.io.IOUtils;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
@@ -59,6 +60,8 @@ public synchronized void persist(String key, String value) {
             _properties.store(output, _name);
             output.flush();
             output.close();
+            AgentPropertiesFileHandler.clearCache();
+            logger.debug("Cleared agent properties cache after persisting key: {}", key);
         } catch (IOException e) {
             logger.error("Uh-oh: ", e);
         } finally {

agent/src/main/java/com/cloud/agent/properties/AgentPropertiesFileHandler.java

@@ -17,6 +17,7 @@
 import com.cloud.utils.PropertiesUtil;
 import java.io.File;
 import java.io.IOException;
+import java.util.Properties;
 import org.apache.cloudstack.utils.security.KeyStoreUtils;
 import org.apache.commons.beanutils.ConvertUtils;
 import org.apache.commons.beanutils.converters.IntegerConverter;
@@ -29,13 +30,18 @@
  * This class provides a facility to read the agent's properties file and get
  * its properties, according to the {@link AgentProperties} properties constants.
  *
+ * Properties are loaded lazily and cached until the cache is cleared (for example, after persisting updates).
  */
 public class AgentPropertiesFileHandler {
 
     protected static Logger LOGGER = LogManager.getLogger(AgentPropertiesFileHandler.class);
 
+    // Simple singleton caching - loaded once, cached forever
+    private static volatile Properties cachedProperties;
+
     /**
      * This method reads the property in the agent.properties file.
+     * Properties are loaded once and cached for the agent's lifetime.
      *
      * @param property the property to retrieve.
      * @return The value of the property. If the property is not available, the
@@ -45,16 +51,15 @@ public static <T> T getPropertyValue(AgentProperties.Property<T> property) {
         T defaultValue = property.getDefaultValue();
         String name = property.getName();
 
-        File agentPropertiesFile = PropertiesUtil.findConfigFile(KeyStoreUtils.AGENT_PROPSFILE);
-
-        if (agentPropertiesFile == null) {
-            LOGGER.debug("File [{}] was not found, we will use default defined values. Property [{}]: [{}].", KeyStoreUtils.AGENT_PROPSFILE, name, defaultValue);
+        Properties properties = getCachedProperties();
 
+        if (properties == null) {
+            LOGGER.debug(String.format("Properties file was not found or could not be loaded, using default values. Property [%s]: [%s].", name, defaultValue));
             return defaultValue;
         }
 
         try {
-            String configValue = PropertiesUtil.loadFromFile(agentPropertiesFile).getProperty(name);
+            String configValue = properties.getProperty(name);
             if (StringUtils.isBlank(configValue)) {
                 LOGGER.debug("Property [{}] has empty or null value. Using default value [{}].", name, defaultValue);
                 return defaultValue;
@@ -71,11 +76,69 @@ public static <T> T getPropertyValue(AgentProperties.Property<T> property) {
             LOGGER.debug("Property [{}] was altered. Now using the value [{}].", name, configValue);
             return (T)ConvertUtils.convert(configValue, property.getTypeClass());
 
-        } catch (IOException ex) {
+        } catch (RuntimeException ex) {
             LOGGER.debug("Failed to get property [{}]. Using default value [{}].", name, defaultValue, ex);
         }
 
         return defaultValue;
     }
 
+    /**
+     * Gets the cached properties, loading them once if not already loaded.
+     * Agent properties are static configuration that don't change during runtime.
+     *
+     * @return cached Properties object or null if file cannot be loaded
+     */
+    private static Properties getCachedProperties() {
+        Properties properties = cachedProperties;
+        if (properties == null) {
+            synchronized (AgentPropertiesFileHandler.class) {
+                properties = cachedProperties;
+                if (properties == null) {
+                    loadProperties();
+                    properties = cachedProperties;
+                }
+            }
+        }
+        return properties;
+    }
+
+    /**
+     * Loads properties from file and caches them for the agent's lifetime.
+     */
+    private static void loadProperties() {
+        File agentPropertiesFile = PropertiesUtil.findConfigFile(KeyStoreUtils.AGENT_PROPSFILE);
+
+        if (agentPropertiesFile == null) {
+            LOGGER.debug("File [{}] was not found.", KeyStoreUtils.AGENT_PROPSFILE);
+            return;
+        }
+
+        try {
+            Properties newProperties = PropertiesUtil.loadFromFile(agentPropertiesFile);
+            cachedProperties = newProperties;
+
+            LOGGER.info("Loaded {} properties from [{}]", newProperties.size(), agentPropertiesFile.getAbsolutePath());
+
+        } catch (IOException ex) {
+            LOGGER.error("Failed to load properties from file [{}].", agentPropertiesFile.getAbsolutePath(), ex);
+        }
+    }
+
+    /**
+     * Clears the properties cache.
+     */
+    public static synchronized void clearCache() {
+        LOGGER.info("Clearing agent properties cache");
+        cachedProperties = null;
+    }
+
+    /**
+     * Returns whether the properties cache is currently loaded.
+     *
+     * @return true if properties are cached, false otherwise.
+     */
+    public static boolean isCacheLoaded() {
+        return cachedProperties != null;
+    }
 }

agent/src/test/java/com/cloud/agent/properties/AgentPropertiesFileHandlerTest.java

@@ -58,12 +58,14 @@ public class AgentPropertiesFileHandlerTest extends TestCase {
     @Before
     public void setUp() throws Exception {
         propertiesUtilMocked = Mockito.mockStatic(PropertiesUtil.class);
+        AgentPropertiesFileHandler.clearCache();
     }
 
     @Override
     @After
     public void tearDown() throws Exception {
         propertiesUtilMocked.close();
+        AgentPropertiesFileHandler.clearCache();
     }
 
     @Test
@@ -196,4 +198,39 @@ public void getPropertyValueTestValidLongPropertyReturnPropertyValue() throws Ex
         Assert.assertEquals(expectedResult, result);
     }
 
+    @Test
+    public void testCacheFunctionality() throws Exception {
+        String expectedResult = "cached-value";
+        AgentProperties.Property<String> agentProperty = new AgentProperties.Property<String>("test.property", "default", String.class);
+
+        propertiesUtilMocked.when(() -> PropertiesUtil.findConfigFile(Mockito.anyString())).thenReturn(fileMock);
+        propertiesUtilMocked.when(() -> PropertiesUtil.loadFromFile(Mockito.any())).thenReturn(propertiesMock);
+        Mockito.doReturn(expectedResult).when(propertiesMock).getProperty(Mockito.anyString());
+
+        Assert.assertFalse("Cache should be empty initially", AgentPropertiesFileHandler.isCacheLoaded());
+
+        String result1 = AgentPropertiesFileHandler.getPropertyValue(agentProperty);
+        Assert.assertEquals("First call should return correct value", expectedResult, result1);
+        Assert.assertTrue("Cache should be loaded after first call", AgentPropertiesFileHandler.isCacheLoaded());
+
+        Mockito.verify(PropertiesUtil.class, Mockito.times(1));
+        PropertiesUtil.loadFromFile(Mockito.any(File.class));
+
+        String result2 = AgentPropertiesFileHandler.getPropertyValue(agentProperty);
+        Assert.assertEquals("Second call should return same cached value", expectedResult, result2);
+        Assert.assertTrue("Cache should still be loaded", AgentPropertiesFileHandler.isCacheLoaded());
+
+        Mockito.verify(PropertiesUtil.class, Mockito.times(1));
+        PropertiesUtil.loadFromFile(Mockito.any(File.class));
+
+        AgentPropertiesFileHandler.clearCache();
+        Assert.assertFalse("Cache should be empty after clear", AgentPropertiesFileHandler.isCacheLoaded());
+
+        String result3 = AgentPropertiesFileHandler.getPropertyValue(agentProperty);
+        Assert.assertEquals("Third call should return correct value after cache clear", expectedResult, result3);
+        Assert.assertTrue("Cache should be loaded again", AgentPropertiesFileHandler.isCacheLoaded());
+
+        Mockito.verify(PropertiesUtil.class, Mockito.times(2));
+        PropertiesUtil.loadFromFile(Mockito.any(File.class));
+    }
 }