Copilot commented on code in PR #9318:
URL: https://github.com/apache/ozone/pull/9318#discussion_r2601255598
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/ServerUtils.java:
##########
@@ -300,11 +304,185 @@ public static String getRemoteUserName() {
return remoteUser != null ? remoteUser.getUserName() : null;
}
- public static String getDefaultRatisDirectory(ConfigurationSource conf) {
+ /**
+ * Get the default Ratis directory for a component when the specific
+ * configuration is not set. This creates a component-specific subdirectory
+ * under ozone.metadata.dirs to avoid conflicts when multiple components
+ * are colocated on the same host.
+ *
+ * <p>For backward compatibility during upgrades, this method checks for
+ * existing Ratis data in old locations before using the new
component-specific
+ * location. See {@link #findExistingRatisDirectory} for details on old
locations.
+ *
+ * @param conf Configuration source
+ * @param nodeType Type of the node component
+ * @return Path to the component-specific ratis directory
+ */
+ public static String getDefaultRatisDirectory(ConfigurationSource conf,
+ HddsProtos.NodeType nodeType) {
LOG.warn("Storage directory for Ratis is not configured. It is a good " +
"idea to map this to an SSD disk. Falling back to {}",
HddsConfigKeys.OZONE_METADATA_DIRS);
File metaDirPath = ServerUtils.getOzoneMetaDirPath(conf);
- return (new File(metaDirPath, "ratis")).getPath();
+
+ // Check for existing Ratis data from old versions for backward
compatibility
+ String existingDir = findExistingRatisDirectory(metaDirPath, nodeType);
+ if (existingDir != null) {
+ return existingDir;
+ }
+
+ // Use new component-specific location for new installations
+ String componentName = getComponentName(nodeType);
+ return (new File(metaDirPath, componentName + ".ratis")).getPath();
+ }
+
+ /**
+ * Checks for existing Ratis directories from previous versions for backward
+ * compatibility during upgrades.
+ *
+ * <p>Older versions of Ozone used different directory structures:
+ * <ul>
+ * <li>Versions up to 2.0.0: Shared {@code <ozone.metadata.dirs>/ratis}
for all components</li>
+ * <li>Some SCM versions: Used {@code <ozone.metadata.dirs>/scm-ha}</li>
+ * </ul>
+ *
+ * @param metaDirPath The ozone metadata directory path
+ * @param nodeType Type of the node component
+ * @return Path to existing old Ratis directory if found, null otherwise
+ */
+ private static String findExistingRatisDirectory(File metaDirPath,
+ NodeType nodeType) {
+ // Check old Ratis directory locations in order of precedence
+ String[] oldRatisDirs = getOldRatisDirectoryNames(nodeType);
+ for (String dirName : oldRatisDirs) {
+ File oldRatisDir = new File(metaDirPath, dirName);
+ if (isNonEmptyDirectory(oldRatisDir)) {
+ LOG.info("Found existing Ratis directory at old location: {}. " +
+ "Using it for backward compatibility during upgrade.",
+ oldRatisDir.getPath());
+ return oldRatisDir.getPath();
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Returns array of old Ratis directory names to check, in order of
precedence.
+ *
+ * @param nodeType Type of the node component
+ * @return Array of directory names to check
+ */
+ private static String[] getOldRatisDirectoryNames(NodeType nodeType) {
+ // Component-specific old locations take precedence over shared locations
+ if (nodeType == NodeType.SCM) {
+ return new String[]{"scm-ha", "ratis"};
+ }
+ return new String[]{"ratis"};
+ }
+
+ /**
+ * Converts NodeType enum to the component name string used for directory
naming.
+ *
+ * @param nodeType Type of the node component
+ * @return Component name string (e.g., "om", "scm", "dn", "recon")
+ */
+ private static String getComponentName(NodeType nodeType) {
+ switch (nodeType) {
+ case OM:
+ return "om";
+ case SCM:
+ return "scm";
+ case DATANODE:
+ return "dn";
Review Comment:
[nitpick] The component name 'dn' is abbreviated while other components use
full names ('om', 'scm', 'recon'). This inconsistency could cause confusion.
Consider using 'datanode' for consistency with the NodeType.DATANODE enum value.
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/TestServerUtils.java:
##########
@@ -263,4 +266,212 @@ public void ozoneMetadataDirRejectsList() {
() -> ServerUtils.getOzoneMetaDirPath(conf));
}
+ /**
+ * Test that SCM, OM, and Datanode colocated on the same host with only
+ * ozone.metadata.dirs configured don't conflict with Ratis directories.
+ */
+ @Test
+ public void testColocatedComponentsWithSharedMetadataDir() {
+ final File metaDir = new File(folder.toFile(), "sharedMetaDir");
+ final OzoneConfiguration conf = new OzoneConfiguration();
+
+ // Only configure ozone.metadata.dirs (the fallback config)
+ conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, metaDir.getPath());
+
+ try {
+ assertFalse(metaDir.exists());
+
+ // Test Ratis directories - each component should get its own with flat
naming
+ String scmRatisDir = ServerUtils.getDefaultRatisDirectory(conf,
HddsProtos.NodeType.SCM);
+ String omRatisDir = ServerUtils.getDefaultRatisDirectory(conf,
HddsProtos.NodeType.OM);
+ String dnRatisDir = ServerUtils.getDefaultRatisDirectory(conf,
HddsProtos.NodeType.DATANODE);
+
+ // Verify Ratis directories use flat naming pattern (component.ratis)
+ assertEquals(new File(metaDir, "scm.ratis").getPath(), scmRatisDir);
+ assertEquals(new File(metaDir, "om.ratis").getPath(), omRatisDir);
+ assertEquals(new File(metaDir, "dn.ratis").getPath(), dnRatisDir);
Review Comment:
[nitpick] The directory name 'dn.ratis' uses an abbreviated component name
('dn') while SCM and OM use their full names ('scm', 'om'). For consistency,
consider using 'datanode.ratis' to match the pattern or ensure this
abbreviation is intentionally chosen and documented.
```suggestion
assertEquals(new File(metaDir, "datanode.ratis").getPath(),
dnRatisDir);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
