Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -72,6 +73,11 @@ public class OzoneConfiguration extends Configuration implements MutableConfigur
activate();
}

/** Ozone's built-in default resources; a value coming only from these may be overridden. */
private static final Set<String> DEFAULT_RESOURCES = getConfigurationResourceFiles().stream()
.filter(resource -> resource.endsWith("-default.xml"))
.collect(Collectors.toSet());

private Properties delegatingProps;

public static OzoneConfiguration of(ConfigurationSource source) {
Expand Down Expand Up @@ -446,6 +452,38 @@ public synchronized void reloadConfiguration() {
delegatingProps = null;
}

/**
* Sets {@code value} unless the property was already set explicitly
* (programmatically, from the command line, from a {@code *-site.xml}, or from a
* user-provided resource). Values that come only from Ozone's built-in default
* resources ({@code *-default.xml}) are overridden.
* <p>
* Hadoop {@link Configuration#setIfUnset(String, String)} uses {@code get(name) == null},
* which never succeeds for keys present in default resources after HDDS-12777.
*/
@Override
public synchronized void setIfUnset(String name, String value) {
if (!isExplicitlySet(name)) {
set(name, value);
}
}

private boolean isExplicitlySet(String name) {
String[] sources = getPropertySources(name);
if (sources == null) {
return false;
}
for (String source : sources) {
// Explicit unless the value comes only from one of Ozone's built-in default
// resources. A user-provided resource is not in that set and is preserved,
// even if it happens to be named *-default.xml.
if (source != null && !DEFAULT_RESOURCES.contains(source)) {
return true;
}
}
return false;
}

@Override
protected final synchronized Properties getProps() {
if (delegatingProps == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -301,6 +302,67 @@ public void testInstantiationWithInputConfiguration(@TempDir File tempDir)
assertNotEquals(val, new OzoneConfiguration().get(key));
}

@Test
public void setIfUnsetOverridesDefaultButKeepsExplicitValue() {
final String key = OZONE_SCM_HANDLER_COUNT_KEY;
OzoneConfiguration subject = new OzoneConfiguration();

// Default resources provide a value, so Hadoop's get(key) is non-null.
assertNotNull(subject.get(key));
String fromDefaults = subject.get(key);

subject.setIfUnset(key, "20");
assertEquals("20", subject.get(key));
assertNotEquals(fromDefaults, subject.get(key));

subject.set(key, "42");
subject.setIfUnset(key, "20");
assertEquals("42", subject.get(key));
}

@Test
public void setIfUnsetPreservesSiteXmlValue(@TempDir File tempDir)
throws IOException {
final String key = OZONE_SCM_HANDLER_COUNT_KEY;
File ozoneSite = new File(tempDir, "ozone-site.xml");
try (BufferedWriter out = new BufferedWriter(new OutputStreamWriter(
Files.newOutputStream(ozoneSite.toPath()), StandardCharsets.UTF_8))) {
startConfig(out);
appendProperty(out, key, "99");
endConfig(out);
}

OzoneConfiguration subject = new OzoneConfiguration();
subject.addResource(new Path(ozoneSite.getAbsolutePath()));
assertEquals("99", subject.get(key));

subject.setIfUnset(key, "20");
assertEquals("99", subject.get(key));
}

@Test
public void setIfUnsetPreservesCustomResourceValue(@TempDir File tempDir)
throws IOException {
final String key = OZONE_SCM_HANDLER_COUNT_KEY;
// Named *-default.xml on purpose: a user-provided resource is explicit even
// when its name matches the built-in default resource convention.
File custom = new File(tempDir, "custom-default.xml");
try (BufferedWriter out = new BufferedWriter(new OutputStreamWriter(
Files.newOutputStream(custom.toPath()), StandardCharsets.UTF_8))) {
startConfig(out);
appendProperty(out, key, "77");
endConfig(out);
}

OzoneConfiguration subject = new OzoneConfiguration();
subject.addResource(new Path(custom.getAbsolutePath()));
assertEquals("77", subject.get(key));

// A value from any non-default resource is explicit and must be preserved.
subject.setIfUnset(key, "20");
assertEquals("77", subject.get(key));
}

@Test
public void setConfigFromObjectWithObjectDefaults() {
// GIVEN
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,71 @@

package org.apache.hadoop.hdds.conf;

import java.io.IOException;
import java.util.Collection;

/**
* Configuration that can be both read and written.
*/
public interface MutableConfigurationSource
extends ConfigurationSource, ConfigurationTarget {

/**
* Sets {@code value} for {@code key} only if the key is not already set.
* Default implementation treats any non-null {@link #get(String)} result as set.
* {@code OzoneConfiguration} (in hdds-common) overrides this to allow
* overriding values that come only from default resources.
*/
default void setIfUnset(String key, String value) {
if (get(key) == null) {
set(key, value);
}
}

/**
* Creates a wrapper config that changes {@link #set(String, String)} to
* {@link #setIfUnset(String, String)}. In other words, value is stored only if
* no existing value is explicitly set.
*/
static MutableConfigurationSource ifUnsetWrapper(MutableConfigurationSource wrapped) {
return new IfUnsetWrapper(wrapped);
}

/**
* Delegates all calls to another configuration object, but changes semantics of
* {@link #set(String, String)} to {@link #setIfUnset(String, String)}.
*/
class IfUnsetWrapper implements MutableConfigurationSource {

private final MutableConfigurationSource wrapped;

private IfUnsetWrapper(MutableConfigurationSource wrapped) {
this.wrapped = wrapped;
}

@Override
public String get(String key) {
return wrapped.get(key);
}

@Override
public Collection<String> getConfigKeys() {
return wrapped.getConfigKeys();
}

@Override
public char[] getPassword(String key) throws IOException {
return wrapped.getPassword(key);
}

@Override
public void set(String key, String value) {
wrapped.setIfUnset(key, value);
}

@Override
public void setIfUnset(String key, String value) {
wrapped.setIfUnset(key, value);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,90 +26,15 @@
<name>ozone.om.s3.grpc.server_enabled</name>
<value>false</value>
</property>
<property>
<name>hdds.container.ratis.num.write.chunk.threads.per.volume</name>
<value>4</value>
</property>
<property>
<name>ozone.scm.handler.count.key</name>
<value>20</value>
</property>
<property>
<name>ozone.om.handler.count.key</name>
<value>20</value>
</property>
<property>
<name>hdds.container.ratis.datastream.enabled</name>
<value>true</value>
</property>
<property>
<name>hdds.heartbeat.interval</name>
<value>1s</value>
</property>
<property>
<name>ozone.scm.heartbeat.thread.interval</name>
<value>100ms</value>
</property>
<property>
<name>ozone.scm.ratis.pipeline.limit</name>
<value>3</value>
</property>
<property>
<name>ozone.scm.close.container.wait.duration</name>
<value>1s</value>
</property>
<property>
<name>ozone.om.snapshot.diff.job.default.wait.time</name>
<value>1s</value>
</property>
<property>
<name>hdds.container.ratis.log.appender.queue.byte-limit</name>
<value>32MB</value>
</property>
<property>
<name>ozone.om.ratis.log.appender.queue.byte-limit</name>
<value>4MB</value>
</property>
<property>
<name>ozone.scm.ha.ratis.log.appender.queue.byte-limit</name>
<value>4MB</value>
</property>
<property>
<name>ozone.scm.chunk.size</name>
<value>1MB</value>
</property>
<property>
<name>ozone.scm.block.size</name>
<value>4MB</value>
</property>
<!-- Keep a larger container than MiniOzoneCluster defaults for Recon tests. -->
<property>
<name>ozone.scm.container.size</name>
<value>128MB</value>
</property>
<property>
<name>ozone.client.stream.buffer.flush.size</name>
<value>1MB</value>
</property>
<property>
<name>ozone.client.stream.buffer.max.size</name>
<value>2MB</value>
</property>
<property>
<name>ozone.client.stream.buffer.size</name>
<value>1MB</value>
</property>
<property>
<name>ozone.client.datastream.buffer.flush.size</name>
<value>4MB</value>
</property>
<property>
<name>ozone.client.datastream.min.packet.size</name>
<value>256KB</value>
</property>
<property>
<name>ozone.client.datastream.window.size</name>
<value>8MB</value>
</property>
<property>
<name>hdds.datanode.volume.min.free.space</name>
<value>5GB</value>
Expand Down
Loading
Loading