adoroszlai opened a new pull request, #8475:
URL: https://github.com/apache/ozone/pull/8475

   ## What changes were proposed in this pull request?
   
   Currently `OzoneConfiguration` loads all `ozone-default-generated.xml` files 
found on the classpath.  It needs to do so, because such files are present in 
multiple modules (`hdds-common`, `ozone-manager`, etc.), and each file only has 
a subset of config properties.  Both `ozone-default.xml` and 
`ozone-default-generated.xml` are added via `addResource`, because 
`addDefaultResource` does not allow multiple resources with the same name in 
different locations (see 
https://github.com/apache/hadoop/pull/773#discussion_r278692062).  However, 
loading _all_ resources produces unexpected results if the classpath contains 
multiple versions of the same jar.
   
   This patch changes Ozone to use module-specific names for the generated 
config files, so we can avoid iterating resources.
   
   - Add a build step to rename `ozone-default-generated.xml` after compilation 
of each module.
   - Remove config file generation from modules that do not have such configs 
currently (`ozone-s3gateway`, `ozone-client`, `ozone-interface-storage`).
   - Move `ReconSqlDbConfig` to `ozone-recon`, where it's used.  This allows 
skipping config generation in `ozone-reconcodegen`.
   - Remove test-specific config class from `ozone-insight`, so we can skip 
config generation in that module, too.
   - Add resources in `OzoneConfiguration` via `addDefaultResource` instead of 
`addResource`.  This is how `Configuration` and `HdfsConfiguration` add their 
own configs, and is what `OzoneConfiguration` used to do before HDDS-1469.  
`addDefaultResource` also handles duplicate names, so duplicate jars should not 
result in overridden config.  As a side-effect:
     - Remove logic from `OzoneConfiguration` related to isolated classloader 
(HDDS-922), no longer needed after HDDS-3458.
     - Remove the hack introduced by HDDS-3871 for looking for Ozone config 
properties in `core-site.xml`.  I think this is consistent with HDFS, whose 
properties should be set in 
[`hdfs-site.xml`](https://github.com/apache/hadoop/blob/626b227094027ed08883af97a0734d2db7863864/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/HdfsConfiguration.java#L32-L41),
 not `core-site.xml`:
   
         ```java
         public class HdfsConfiguration extends Configuration {
           static {
             addDeprecatedKeys();
   
   
             // adds the default resources
             Configuration.addDefaultResource("hdfs-default.xml");
             Configuration.addDefaultResource("hdfs-rbf-default.xml");
             Configuration.addDefaultResource("hdfs-site.xml");
             Configuration.addDefaultResource("hdfs-rbf-site.xml");
           }
         ```
   
   - Minor HttpFS fixes (included because HttpFS tests were timing out with the 
initial version of the patch):
     - Use Ozone's startup message for HttpFS to show Ozone version and 
configuration.
     - Set timeout for HttpFS acceptance tests.
   
   https://issues.apache.org/jira/browse/HDDS-12777
   
   ## How was this patch tested?
   
   Checked OM and SCM configuration in Docker Compose cluster:
   
   ```bash
   cd hadoop-ozone/dist/target/ozone-2.1.0-SNAPSHOT/compose/ozone
   OZONE_DATANODES=3 ./run.sh -d
   open http://localhost:9874/conf
   open http://localhost:9876/conf
   ```
   
   Verified that items from various config files are present, e.g.:
   
   ```xml
   <property>
     <name>ozone.scm.dead.node.interval</name>
     <value>45s</value>
     <final>false</final>
     <source>ozone-site.xml</source>
   </property>
   <property>
     <name>hdds.container.ipc.random.port</name>
     <value>false</value>
     <final>false</final>
     <source>ozone-default.xml</source>
   </property>
   <property>
     <name>hdds.scm.block.deletion.txn.dn.commit.map.limit</name>
     <value>5000000</value>
     <final>false</final>
     <source>hdds-client-default.xml</source>
   </property>
   <property>
     <name>ipc.server.tcpnodelay</name>
     <value>true</value>
     <final>false</final>
     <source>core-default.xml</source>
   </property>
   <property>
     <name>hdds.container.balancer.move.timeout</name>
     <value>65m</value>
     <final>false</final>
     <source>hdds-common-default.xml</source>
   </property>
   <property>
     <name>hadoop.hdds.db.rocksdb.writeoption.sync</name>
     <value>false</value>
     <final>false</final>
     <source>hdds-server-framework-default.xml</source>
   </property>
   ```
   
   Only OM has items from `ozone-manager-default.xml`:
   
   ```xml
   <property>
     <name>ozone.om.user.max.volume</name>
     <value>1024</value>
     <final>false</final>
     <source>ozone-manager-default.xml</source>
   </property>
   ```
   
   And only SCM has items from `hdds-server-scm-default.xml`:
   
   ```xml
   <property>
     <name>hdds.scm.replication.datanode.delete.container.limit</name>
     <value>40</value>
     <final>false</final>
     <source>hdds-server-scm-default.xml</source>
   </property>
   ```
   
   CI:
   https://github.com/adoroszlai/ozone/actions/runs/15083080061


-- 
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]

Reply via email to