Skip to content

Commit

Permalink
[router] Made most VeniceRouterConfig fields final (#1405)
Browse files Browse the repository at this point in the history
There are only two exceptions, related to throttling, which are allowed
to be mutated post-init.

Miscellaneous:

- Deleted a dead config: router.compute.fast.avro.enabled
- Made VeniceProperties::getMap return an unmodifiable map.
- Flaky unit test tweaks (unrelated to this change).
  • Loading branch information
FelixGV authored Dec 19, 2024
1 parent 543f65c commit cc10770
Show file tree
Hide file tree
Showing 7 changed files with 364 additions and 309 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -427,26 +427,22 @@ public Map<String, String> getMap(String key) {
if (!containsKey(key)) {
throw new UndefinedPropertyException(key);
}
Map<String, String> map = new HashMap<>();
List<String> keyValuePairs = this.getList(key);

for (String pair: keyValuePairs) {
Pair<String, String> keyValuePair = splitAnEntryToKeyValuePair(pair);
map.put(keyValuePair.getFirst(), keyValuePair.getSecond());
}
return map;
}

private Pair<String, String> splitAnEntryToKeyValuePair(String entry) {
if (!entry.contains(":")) {
throw new VeniceException("Invalid config. Expect each entry to contain at least one \":\". Got: " + entry);
Map<String, String> map = new HashMap<>(keyValuePairs.size());

for (String keyValuePair: keyValuePairs) {
// One entry could have multiple ":". For example, "<ID>:<Kafka URL>:<port>". In this case, we split the String by
// its first ":" so that we get key=<ID> and value=<Kafka URL>:<port>
int indexOfFirstColon = keyValuePair.indexOf(':');
if (indexOfFirstColon == -1) {
throw new VeniceException(
"Invalid config. Expect each entry to contain at least one \":\". Got: " + keyValuePair);
}
String mapKey = keyValuePair.substring(0, indexOfFirstColon);
String mapValue = keyValuePair.substring(indexOfFirstColon + 1);
map.put(mapKey, mapValue);
}
// One entry could have multiple ":". For example, "<ID>:<Kafka URL>:<port>". In this case, we split the String by
// its first ":" so that we get key=<ID> and value=<Kafka URL>:<port>
int indexOfFirstColon = entry.indexOf(':');
String key = entry.substring(0, indexOfFirstColon);
String value = entry.substring(indexOfFirstColon + 1);
return new Pair<>(key, value);
return Collections.unmodifiableMap(map);
}

public Properties toProperties() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package com.linkedin.venice.utils;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertThrows;

import com.linkedin.venice.exceptions.VeniceException;
import java.util.Map;
import java.util.Properties;
import org.testng.Assert;
import org.testng.annotations.Test;

Expand All @@ -22,12 +23,21 @@ public void testConvertSizeFromLiteral() {

@Test
public void testGetMapWhenMapIsStringEncoded() {
Properties properties = new Properties();
properties.put("region.to.pubsub.broker.map", "prod:https://prod-broker:1234,dev:dev-broker:9876");
VeniceProperties veniceProperties = new VeniceProperties(properties);
VeniceProperties veniceProperties =
new PropertyBuilder().put("region.to.pubsub.broker.map", "prod:https://prod-broker:1234,dev:dev-broker:9876")
.build();
Map<String, String> map = veniceProperties.getMap("region.to.pubsub.broker.map");
assertEquals(map.size(), 2);
assertEquals(map.get("prod"), "https://prod-broker:1234");
assertEquals(map.get("dev"), "dev-broker:9876");

// Map should be immutable
assertThrows(() -> map.put("foo", "bar"));

// Invalid encoding
VeniceProperties invalidVeniceProperties =
new PropertyBuilder().put("region.to.pubsub.broker.map", "prod:https://prod-broker:1234,dev;dev-broker")
.build();
assertThrows(VeniceException.class, () -> invalidVeniceProperties.getMap("region.to.pubsub.broker.map"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1050,11 +1050,6 @@ private ConfigKeys() {

public static final String ROUTER_CLIENT_DECOMPRESSION_ENABLED = "router.client.decompression.enabled";

/**
* Whether to enable fast-avro in router;
*/
public static final String ROUTER_COMPUTE_FAST_AVRO_ENABLED = "router.compute.fast.avro.enabled";

/**
* Socket timeout config for the connection manager from router to server
*/
Expand Down
Loading

0 comments on commit cc10770

Please sign in to comment.