Skip to content

Conversation

@crliao
Copy link

@crliao crliao commented Dec 16, 2025

Summary

  • Populate instance config during auto registration to Helix
  • Populate data node config during to property store, essentially to ZK

Testing Done

  • UT

Comment on lines 423 to 432
@Config("clustermap.nimbus.service.metadata.file.path")
@Default("./etc/metadata/nimbus-service.json")
public final String nimbusServiceMetadataFilePath;

/**
* Path to the LiStatefulSet metadata file containing Kubernetes StatefulSet information
*/
@Config("clustermap.listatefulset.metadata.file.path")
@Default("./etc/metadata/liStatefulSet.json")
public final String liStatefulSetMetadataFilePath;
Copy link
Contributor

Choose a reason for hiding this comment

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

These values are internal linkedin concepts, please keep them out of the open source repo. We can define them in the AmbryLI configs instead.

// Pattern to match df -h output lines for Ambry mount points
// Example: /dev/sdh1 21T 14T 6.5T 68% /mnt/u001/ambrydata
private static final Pattern DF_PATTERN = Pattern.compile(
"^(\\S+)\\s+(\\S+)\\s+(\\S+)\\s+(\\S+)\\s+(\\d+)%\\s+(/mnt/u\\d+/ambrydata)$");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this pattern configurable too?

Comment on lines +151 to +155
/**
* Collect disk information by running 'df -h' command.
* @return map of mount point to DiskInfo
*/
public static Map<String, DiskInfo> collectDiskInfo() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we should use some library like OSHI instead of running df -h

Comment on lines +122 to +129
NimbusServiceMetadata nimbusMetadata = NimbusServiceMetadata.readFromFile(clusterMapConfig.nimbusServiceMetadataFilePath);
if (nimbusMetadata != null) {
LOGGER.info("Loaded nimbus service metadata - AppInstanceID: {}, NodeName: {}, MaintenanceZone: {}",
nimbusMetadata.getAppInstanceID(), nimbusMetadata.getNodeName(), nimbusMetadata.getMaintenanceZone());
instanceConfigBuilder.setDomain(String.format(DOMAIN_TEMPLATE, nimbusMetadata.getMaintenanceZone(), nimbusMetadata.getNodeName(), nimbusMetadata.getAppInstanceID()));
}

LiStatefulSetMetadata liStatefulSetMetadata = LiStatefulSetMetadata.readFromFile(clusterMapConfig.liStatefulSetMetadataFilePath);
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to earlier we will need to keep internal LI concepts out since this is a generic open-source repo. We will either need to extend the HelixFactory in AmbryLI or convert this class to some interface that we can implement instead.

@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2025

Codecov Report

❌ Patch coverage is 3.77358% with 204 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.29%. Comparing base (52ba813) to head (76061a1).
⚠️ Report is 333 commits behind head on master.

Files with missing lines Patch % Lines
...com/github/ambry/clustermap/DiskInfoCollector.java 0.00% 89 Missing ⚠️
...github/ambry/clustermap/LiStatefulSetMetadata.java 0.00% 37 Missing ⚠️
...java/com/github/ambry/clustermap/HelixFactory.java 0.00% 22 Missing ⚠️
...ls/src/main/java/com/github/ambry/utils/Utils.java 5.55% 17 Missing ⚠️
.../com/github/ambry/clustermap/HelixParticipant.java 0.00% 16 Missing ⚠️
...va/com/github/ambry/clustermap/DataNodeConfig.java 0.00% 12 Missing ⚠️
...github/ambry/clustermap/NimbusServiceMetadata.java 0.00% 6 Missing ⚠️
...main/java/com/github/ambry/server/AmbryServer.java 0.00% 2 Missing ⚠️
...y/clustermap/RecoveryTestClusterAgentsFactory.java 0.00% 1 Missing ⚠️
...b/ambry/clustermap/StaticClusterAgentsFactory.java 0.00% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #3188       +/-   ##
=============================================
- Coverage     64.24%   51.29%   -12.95%     
+ Complexity    10398     8650     -1748     
=============================================
  Files           840      933       +93     
  Lines         71755    79218     +7463     
  Branches       8611     9457      +846     
=============================================
- Hits          46099    40635     -5464     
- Misses        23004    35241    +12237     
- Partials       2652     3342      +690     

☔ View full report in Codecov by Sentry.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants