[ 
https://issues.apache.org/jira/browse/HDDS-17636?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Andrey Yarovoy resolved HDDS-17636.
-----------------------------------
    Resolution: Fixed

> ReplicationConfig.parse reads ozone.replication even when a replication is 
> passed
> ---------------------------------------------------------------------------------
>
>                 Key: HDDS-17636
>                 URL: https://issues.apache.org/jira/browse/HDDS-17636
>             Project: Apache Ozone
>          Issue Type: Improvement
>          Components: common
>            Reporter: Andrey Yarovoy
>            Assignee: Andrey Yarovoy
>            Priority: Major
>
> `ReplicationConfig.parse` falls back to `ozone.replication` when no 
> replication is given:
> ```java
> replication = Objects.toString(replication,
> config.get(OZONE_REPLICATION, OZONE_REPLICATION_DEFAULT));
> ```
> Java evaluates the fallback argument first, so the configuration is read on 
> every call, including when `replication` is
> not null and the value is thrown away.
> This is on the path of every ofs/o3fs `create()`: 
> `OzoneClientUtils.resolveClientSideReplicationConfig` calls
> `ReplicationConfig.adjustReplication` or `parse` with the replication factor 
> from the FileSystem API, so it is never
> null there.
> The read is cheap on one thread, but Hadoop's `Configuration.get` goes 
> through `getProps()`, which is `synchronized`.
> A `FileSystem` instance shares one configuration across all its threads, so 
> every create takes that lock once for
> this read. After HDDS-17569, each `parse` takes it twice: once here, and once 
> for `ozone.replication.allowed-configs`
> in the validator. The unneeded read is half of the lock acquisitions.
> **Proposal:** read the configuration only when `replication` is null:
> ```java
> if (replication == null) {
> replication = config.get(OZONE_REPLICATION, OZONE_REPLICATION_DEFAULT);
> }
> ```
> There is no behavior change: the value read when `replication` is not null is 
> never used.
> **Measured** with `ReplicationConfigParseBenchmark` (added for HDDS-17569; 
> `parse(RATIS, "3", conf)`, one
> `OzoneConfiguration` shared by all threads, JDK 21, best of 5 rounds, 2 runs):
> | | after HDDS-17569 | with this change | |
> |---|---|---|---|
> | `parse`, ns/call (1 thread) | 94–97 | 79 | 1.2x |
> | `adjustReplication`, ns/call (1 thread) | 97–102 | 83–87 | 1.2x |
> | `parse` calls/ms, 1 thread | 10,053–10,478 | 12,090–12,136 | 1.2x |
> | `parse` calls/ms, 4 threads | 3,020 | 5,349–5,568 | 1.8x |
> | `parse` calls/ms, 8 threads | 2,939–3,027 | 5,181–5,425 | 1.8x |
> | `parse` calls/ms, 16 threads | 3,041–3,125 | 5,192–5,446 | 1.7x |
> Throughput with several threads stays below one thread's either way, because 
> the remaining read of
> `ozone.replication.allowed-configs` still takes the `Configuration` lock. 
> Removing that one too would need the validator
> to stop following configuration changes, or the Hadoop fix, so it is out of 
> scope here.



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to