Make setting jute max buffer testable and add test

2 files changed, 38 insertions, 10 deletions

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<Path, Lock> 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<File> 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<RetryPolicy, CuratorFramework> 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<File> clientConfigFile) {
+    Curator(ConnectionSpec connectionSpec, Optional<File> 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);
     }
 
 }