Skip to content

Performance improvements of AgentPropertiesFileHandler - #13852

Merged
nvazquez merged 1 commit into
apache:mainfrom
shapeblue:agent-properties-handler-improvements
Nov 9, 2026
Merged

nvazquez merged 1 commit into
apache:mainfrom
shapeblue:agent-properties-handler-improvements

Conversation

@sureshanaparti

Copy link
Copy Markdown
Contributor

Description

This PR optimizes the AgentPropertiesFileHandler.getPropertyValue method. This method is used to fetch properties values from agent.properties file. In the proposed change, the properties file is loaded lazily on first access and cached. The cache is invalidated whenever the agent persists a change via PropertiesStorage.persist, so agent-driven updates are picked up on the next read. External manual edits to agent.properties are not auto-detected and take effect after an agent restart.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@sureshanaparti
sureshanaparti force-pushed the agent-properties-handler-improvements branch from 26e4af3 to 53f7168 Compare August 11, 2026 14:59
@codecov

codecov Bot commented Aug 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 20.78%. Comparing base (aa82348) to head (463ccf8).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...d/agent/properties/AgentPropertiesFileHandler.java 94.44% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              main   #13852       +/-   ##
============================================
+ Coverage     3.88%   20.78%   +16.90%     
- Complexity       0    21942    +21942     
============================================
  Files          493     6496     +6003     
  Lines        42580   591822   +549242     
  Branches      8072    72615    +64543     
============================================
+ Hits          1653   123011   +121358     
- Misses       40696   455460   +414764     
- Partials       231    13351    +13120     
Flag Coverage Δ
uitests 3.88% <ø> (ø)
unittests 22.09% <94.73%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about the failure handling.

LOGGER.info("Loaded {} properties from [{}]", newProperties.size(), agentPropertiesFile.getAbsolutePath());

} catch (IOException ex) {
LOGGER.error("Failed to load properties from file [{}].", agentPropertiesFile.getAbsolutePath(), ex);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems FATAL to me. Any reason to keep running when this occurs

@slavkap slavkap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM

@kiranchavala

Copy link
Copy Markdown
Member

@blueorangutan package

@kiranchavala kiranchavala left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM tested manually

Changes the KVM agent. It adds a cache for agent.properties; the management server is unaffected.

  1. The agent still reads its settings correctly

Before restarting, set a few non-default values in agent.properties. Each one here has an effect you can see:

cat >> /etc/cloudstack/agent/agent.properties <<'EOF'
host.tags=pr855
vm.memballoon.stats.period=10
iothreads=2
EOF

systemctl restart cloudstack-agent


[root@ref-trl-12390-k-Mol8-kiran-chavala-mgmt1 ~]# cmk list hosts type=Routing name=ref-trl-12390-k-Mol8-kiran-chavala-kvm1 filter=name,state,hosttags
{
  "count": 1,
  "host": [
    {
      "hosttags": "pr855",
      "name": "ref-trl-12390-k-Mol8-kiran-chavala-kvm1",
      "state": "Up"
    }
  ]
}



[root@ref-trl-12390-k-Mol8-kiran-chavala-kvm1 ~]# virsh dumpxml i-2-3-VM | grep -E "stats period|<iothreads>"
      <stats period='10'/>

  1. Updates the agent makes itself are picked up

[root@ref-trl-12390-k-Mol8-kiran-chavala-mgmt1 ~]# cmk update configuration name=host value=10.0.33.32
{
  "configuration": {
    "category": "Advanced",
    "component": "ApiServiceConfiguration",
    "defaultvalue": "localhost",
    "description": "The ip address of management server. This can also accept comma separated addresses.",
    "displaytext": "Host",
    "group": "Hypervisor",
    "isdynamic": true,
    "name": "host",
    "subgroup": "Hypervisor",
    "type": "CSV",
    "value": "10.0.33.32"
  }
}


[root@ref-trl-12390-k-Mol8-kiran-chavala-kvm1 ~]# grep "^host=" /etc/cloudstack/agent/agent.properties
host=10.0.33.32@static

[root@ref-trl-12390-k-Mol8-kiran-chavala-kvm1 ~]# grep -E "Processed new management server list|Cleared agent properties cache after persisting key: host" /var/log/cloudstack/agent/agent.log | tail -2
2026-09-25 04:38:39,074 DEBUG [dao.impl.PropertiesStorage] (AgentRequest-Handler-2:null) (logid:eb7ab863) Cleared agent properties cache after persisting key: host
2026-09-25 04:38:39,074 INFO  [cloud.agent.Agent] (AgentRequest-Handler-2:null) (logid:eb7ab863) Processed new management server list: 10.0.33.32@static

  1. How hand edits behave now

sed -i 's/^vm.memballoon.stats.period=.*/vm.memballoon.stats.period=20/' /etc/cloudstack/agent/agent.properties

cmk stop virtualmachine id=<vm>; cmk start virtualmachine id=<vm> hostid=<host-id>


[root@ref-trl-12390-k-Mol8-kiran-chavala-kvm1 ~]# virsh dumpxml i-2-3-VM | grep "stats period"
      <stats period='10'/>

# PR build: still 10 (cached). After a restart, the new value is used:

systemctl restart cloudstack-agent      

[root@ref-trl-12390-k-Mol8-kiran-chavala-kvm1 ~]# virsh dumpxml i-2-3-VM | grep "stats period"
      <stats period='20'/>



@nvazquez

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19345

@nvazquez

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian Build Failed (tid-17042)

@nvazquez

nvazquez commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

@blueorangutan test

@nvazquez

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan

blueorangutan commented Oct 18, 2026 •

Copy link
Copy Markdown

[SF] Trillian test result (tid-17152)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 47228 seconds
Marvin logs: [archive removed]
Smoke tests completed. 149 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@nvazquez nvazquez added this to the 24.0 milestone Nov 9, 2026
@nvazquez
nvazquez force-pushed the agent-properties-handler-improvements branch from 53f7168 to 463ccf8 Compare November 9, 2026 20:35
@nvazquez
nvazquez changed the base branch from 4.22 to main November 9, 2026 20:35
@nvazquez

nvazquez commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19958

@nvazquez
nvazquez merged commit 98b8f75 into apache:main Nov 9, 2026
24 of 26 checks passed
@github-project-automation github-project-automation Bot moved this from conflict/waiting to Done in CloudStack Testing Nov 9, 2026
@nvazquez
nvazquez deleted the agent-properties-handler-improvements branch November 9, 2026 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

7 participants