From 92c22688b15585bc005373f7ee409b13872769aa Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 17 Feb 2022 20:19:09 +0100 Subject: Make setting jute max buffer testable and add test --- .../main/java/com/yahoo/vespa/curator/Curator.java | 23 ++++++++++++-------- .../java/com/yahoo/vespa/curator/CuratorTest.java | 25 +++++++++++++++++++++- 2 files changed, 38 insertions(+), 10 deletions(-) (limited to 'zkfacade') diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java index f96e46d90d8..d226cfcd43e 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -61,22 +61,24 @@ public class Curator implements VespaCurator, AutoCloseable { private static final Duration BASE_SLEEP_TIME = Duration.ofSeconds(1); private static final int MAX_RETRIES = 10; private static final RetryPolicy DEFAULT_RETRY_POLICY = new ExponentialBackoffRetry((int) BASE_SLEEP_TIME.toMillis(), MAX_RETRIES); - private static final long juteMaxBuffer = 52428800; // Should correspond with value in ZookeeperServerConfig + // Default value taken from ZookeeperServerConfig + static final long defaultJuteMaxBuffer = Long.parseLong(System.getProperty("jute.maxbuffer", "52428800")); private final CuratorFramework curatorFramework; private final ConnectionSpec connectionSpec; + private final long juteMaxBuffer; // All lock keys, to allow re-entrancy. This will grow forever, but this should be too slow to be a problem private final ConcurrentHashMap locks = new ConcurrentHashMap<>(); /** Creates a curator instance from a comma-separated string of ZooKeeper host:port strings */ public static Curator create(String connectionSpec) { - return new Curator(ConnectionSpec.create(connectionSpec), Optional.of(ZK_CLIENT_CONFIG_FILE)); + return new Curator(ConnectionSpec.create(connectionSpec), Optional.of(ZK_CLIENT_CONFIG_FILE), defaultJuteMaxBuffer); } // For testing only, use Optional.empty for clientConfigFile parameter to create default zookeeper client config public static Curator create(String connectionSpec, Optional clientConfigFile) { - return new Curator(ConnectionSpec.create(connectionSpec), clientConfigFile); + return new Curator(ConnectionSpec.create(connectionSpec), clientConfigFile, defaultJuteMaxBuffer); } @Inject @@ -87,14 +89,15 @@ public class Curator implements VespaCurator, AutoCloseable { CuratorConfig.Server::hostname, CuratorConfig.Server::port, curatorConfig.zookeeperLocalhostAffinity()), - Optional.of(ZK_CLIENT_CONFIG_FILE)); + Optional.of(ZK_CLIENT_CONFIG_FILE), + defaultJuteMaxBuffer); } protected Curator(String connectionSpec, String zooKeeperEnsembleConnectionSpec, Function curatorFactory) { - this(ConnectionSpec.create(connectionSpec, zooKeeperEnsembleConnectionSpec), curatorFactory.apply(DEFAULT_RETRY_POLICY)); + this(ConnectionSpec.create(connectionSpec, zooKeeperEnsembleConnectionSpec), curatorFactory.apply(DEFAULT_RETRY_POLICY), defaultJuteMaxBuffer); } - Curator(ConnectionSpec connectionSpec, Optional clientConfigFile) { + Curator(ConnectionSpec connectionSpec, Optional clientConfigFile, long juteMaxBuffer) { this(connectionSpec, CuratorFrameworkFactory .builder() @@ -104,12 +107,14 @@ public class Curator implements VespaCurator, AutoCloseable { .connectString(connectionSpec.local()) .zookeeperFactory(new VespaZooKeeperFactory(createClientConfig(clientConfigFile))) .dontUseContainerParents() // TODO: Remove when we know ZooKeeper 3.5 works fine, consider waiting until Vespa 8 - .build()); + .build(), + juteMaxBuffer); } - private Curator(ConnectionSpec connectionSpec, CuratorFramework curatorFramework) { + private Curator(ConnectionSpec connectionSpec, CuratorFramework curatorFramework, long juteMaxBuffer) { this.connectionSpec = Objects.requireNonNull(connectionSpec); this.curatorFramework = Objects.requireNonNull(curatorFramework); + this.juteMaxBuffer = juteMaxBuffer; addLoggingListener(); curatorFramework.start(); } @@ -184,7 +189,7 @@ public class Curator implements VespaCurator, AutoCloseable { public void set(Path path, byte[] data) { if (data.length > juteMaxBuffer) throw new IllegalArgumentException("Cannot not set data at " + path.getAbsolute() + ", " + - data.length + " bytes, max bytes is " + juteMaxBuffer); + data.length + " bytes is too much, max number of bytes allowed per node is " + juteMaxBuffer); if ( ! exists(path)) create(path); diff --git a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java index c4e0cfd3b83..71eae121313 100644 --- a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java +++ b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java @@ -3,11 +3,14 @@ package com.yahoo.vespa.curator; import com.yahoo.cloud.config.CuratorConfig; import com.yahoo.net.HostName; +import com.yahoo.path.Path; +import com.yahoo.text.Utf8; import org.junit.Test; import java.util.Optional; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; /** * Very simple testing of setting up curator @@ -40,6 +43,21 @@ public class CuratorTest { } } + @Test + public void require_that_write_fails_if_data_is_more_than_jute_max_buffer() { + CuratorConfig.Builder builder = new CuratorConfig.Builder(); + builder.server(createZKBuilder(localhost, port1)); + try (Curator curator = createCurator(new CuratorConfig(builder), 1)) { + try { + curator.set(Path.createRoot().append("foo"), Utf8.toBytes("more than 1 byte")); + fail("Did not fail when writing more than juteMaxBuffer bytes"); + } catch (IllegalArgumentException e) { + assertEquals("Cannot not set data at /foo, 16 bytes is too much, max number of bytes allowed per node is 1", + e.getMessage()); + } + } + } + private CuratorConfig createTestConfig() { CuratorConfig.Builder builder = new CuratorConfig.Builder(); builder.server(createZKBuilder(localhost, port1)); @@ -55,11 +73,16 @@ public class CuratorTest { } private Curator createCurator(CuratorConfig curatorConfig) { + return createCurator(curatorConfig, Curator.defaultJuteMaxBuffer); + } + + private Curator createCurator(CuratorConfig curatorConfig, long juteMaxBuffer) { return new Curator(ConnectionSpec.create(curatorConfig.server(), CuratorConfig.Server::hostname, CuratorConfig.Server::port, curatorConfig.zookeeperLocalhostAffinity()), - Optional.empty()); + Optional.empty(), + juteMaxBuffer); } } -- cgit v1.2.3