zk client config update and bugfix for ZKMetadataClientDriver

.github/workflows/backward-compat-tests.yml

@@ -0,0 +1,58 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+name: Backward compatibility tests
+
+on:
+  push:
+  pull_request:
+    branches:
+      - master
+      - branch-*
+    paths-ignore:
+      - 'site/**'
+    workflow_dispatch:
+
+env:
+  MAVEN_OPTS: -Dmaven.wagon.httpconnectionManager.ttlSeconds=25 -Dmaven.wagon.http.retryHandler.count=3
+
+jobs:
+  test:
+
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+
+      - name: Tune Runner VM
+        uses: ./.github/actions/tune-runner-vm
+
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      - name: Build
+        run: ./gradlew stream:server:build -x test
+      - name: Test current server with old clients
+        run: ./gradlew :tests:backward-compat:current-server-old-clients:test || (tail -n +1 tests/backward-compat/current-server-old-clients/build/reports/tests/test/classes/* && tail -n +1 tests/backward-compat/current-server-old-clients/build/container-logs/**/* && exit 1)
+      - name: Test progressive upgrade
+        run: ./gradlew :tests:backward-compat:upgrade:test || (tail -n +1 tests/backward-compat/upgrade/build/reports/tests/test/classes/* && tail -n +1 tests/backward-compat/upgrade/build/container-logs/**/* && exit 1)
+      - name: Other tests
+        run: ./gradlew :tests:backward-compat:test -x tests:backward-compat:upgrade:test -x :tests:backward-compat:current-server-old-clients:test

bin/common_gradle.sh

@@ -139,8 +139,8 @@ CLI_GC_OPTS=${CLI_GC_OPTS:-"${DEFAULT_CLI_GC_OPTS}"}
 CLI_GC_LOGGING_OPTS=${CLI_GC_LOGGING_OPTS:-"${DEFAULT_CLI_GC_LOGGING_OPTS}"}
 
 # module names
-BOOKIE_SERVER_MODULE_NAME="bookkeeper-server"
-TABLE_SERVICE_MODULE_NAME="stream-storage-server"
+BOOKIE_SERVER_MODULE_NAME="(org.apache.bookkeeper-)?bookkeeper-server"
+TABLE_SERVICE_MODULE_NAME="(org.apache.bookkeeper-)?stream-storage-server"
 
 is_released_binary() {
   if [ -d ${BK_HOME}/lib ]; then

bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java

@@ -75,6 +75,7 @@ public abstract class AbstractConfiguration<T extends AbstractConfiguration>
     // Zookeeper Parameters
     protected static final String ZK_TIMEOUT = "zkTimeout";
     protected static final String ZK_SERVERS = "zkServers";
+    protected static final String ZK_RETRY_BACKOFF_MAX_RETRIES = "zkRetryBackoffMaxRetries";
 
     // Ledger Manager
     protected static final String LEDGER_MANAGER_TYPE = "ledgerManagerType";
@@ -345,6 +346,27 @@ public T setZkTimeout(int zkTimeout) {
         return getThis();
     }
 
+    /**
+     * Get zookeeper client backoff max retry times.
+     *
+     * @return zk backoff max retry times.
+     */
+    public int getZkRetryBackoffMaxRetries() {
+        return getInt(ZK_RETRY_BACKOFF_MAX_RETRIES, Integer.MAX_VALUE);
+    }
+
+    /**
+     * Set zookeeper client backoff max retry times.
+     *
+     * @param maxRetries
+     *          backoff max retry times
+     * @return server configuration.
+     */
+    public T setZkRetryBackoffMaxRetries(int maxRetries) {
+        setProperty(ZK_RETRY_BACKOFF_MAX_RETRIES, Integer.toString(maxRetries));
+        return getThis();
+    }
+
     /**
      * Set Ledger Manager Type.
      *

bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/zk/ZKMetadataBookieDriver.java

@@ -56,7 +56,7 @@ public synchronized MetadataBookieDriver initialize(ServerConfiguration conf,
             conf,
             statsLogger.scope(BOOKIE_SCOPE),
             new BoundExponentialBackoffRetryPolicy(conf.getZkRetryBackoffStartMs(),
-                        conf.getZkRetryBackoffMaxMs(), Integer.MAX_VALUE),
+                        conf.getZkRetryBackoffMaxMs(), conf.getZkRetryBackoffMaxRetries()),
             Optional.empty());
         this.serverConf = conf;
         this.statsLogger = statsLogger;

bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/zk/ZKMetadataClientDriver.java

@@ -66,7 +66,7 @@ public synchronized MetadataClientDriver initialize(ClientConfiguration conf,
             new BoundExponentialBackoffRetryPolicy(
                 conf.getZkTimeout(),
                 conf.getZkTimeout(),
-                0),
+                conf.getZkRetryBackoffMaxRetries()),
             optionalCtx);
         this.statsLogger = statsLogger;
         this.clientConf = conf;

build.gradle

@@ -148,6 +148,7 @@ allprojects {
 
         plugins.withType(DistributionPlugin) {
             distTar {
+                archiveClassifier = "bin"
                 compression = Compression.GZIP
                 archiveExtension = 'tar.gz'
             }
@@ -184,7 +185,6 @@ allprojects {
         signing {
             def skipSigning = project.hasProperty('skipSigning') && skipSigning.toBoolean()
             def shouldSign = !skipSigning
-
             if (shouldSign) {
                 if (project.hasProperty("singingKey")) {
                     // The following allow the secretKey and password to be specified using env var
@@ -227,6 +227,12 @@ allprojects {
                     });
                 }
             }
+            systemProperty "gradle.buildDirectory", project.buildDir
+            systemProperty "currentVersion", project.rootProject.version
+            testLogging {
+                outputs.upToDateWhen {false}
+                showStandardStreams = true
+            }
         }
     }
 

settings.gradle

@@ -78,6 +78,7 @@ include(':bookkeeper-benchmark',
         'stream:storage:api',
         'stream:storage:impl',
         'stream:tests-common',
+        'tests:docker-images',
         'tests:backward-compat',
         'tests:backward-compat:bc-non-fips',
         'tests:backward-compat:current-server-old-clients',

stream/server/src/main/java/org/apache/bookkeeper/stream/server/service/RegistrationServiceProvider.java

@@ -64,11 +64,11 @@ public RegistrationServiceProvider(ServerConfiguration bkServerConf,
         this.zkServers = ZKMetadataDriverBase.resolveZkServers(bkServerConf);
         this.regPath = ZK_METADATA_ROOT_PATH + "/" + SERVERS_PATH;
         this.bkZkRetryPolicy = new BoundExponentialBackoffRetryPolicy(
-            bkServerConf.getZkRetryBackoffStartMs(),
-            bkServerConf.getZkRetryBackoffMaxMs(),
-            Integer.MAX_VALUE);
+                bkServerConf.getZkRetryBackoffStartMs(),
+                bkServerConf.getZkRetryBackoffMaxMs(),
+                bkServerConf.getZkRetryBackoffMaxRetries());
         this.regExecutor = Executors.newSingleThreadScheduledExecutor(
-            new ThreadFactoryBuilder().setNameFormat("registration-service-provider-scheduler").build());
+                new ThreadFactoryBuilder().setNameFormat("registration-service-provider-scheduler").build());
         ClientConfiguration clientConfiguration = new ClientConfiguration(bkServerConf);
         this.bookieAddresschangeTracking = clientConfiguration.getEnableBookieAddressTracking();
     }

tests/backward-compat/build.gradle

@@ -38,6 +38,15 @@ subprojects {
         testImplementation depLibs.junit
         testImplementation depLibs.slf4j
         testImplementation depLibs.arquillianCubeDocker
+        testImplementation depLibs.zookeeper
     }
 
+    test {
+        dependsOn(":tests:docker-images:buildImages")
+    }
+
+}
+
+test {
+    subprojects.each { dependsOn("${it.name}:test") }
 }

tests/backward-compat/current-server-old-clients/src/test/groovy/org/apache/bookkeeper/tests/backwardcompat/AbstractClientFencingTest.groovy

@@ -0,0 +1,97 @@
+/*
+* Licensed to the Apache Software Foundation (ASF) under one
+* or more contributor license agreements.  See the NOTICE file
+* distributed with this work for additional information
+* regarding copyright ownership.  The ASF licenses this file
+* to you under the Apache License, Version 2.0 (the
+* "License"); you may not use this file except in compliance
+* with the License.  You may obtain a copy of the License at
+*
+*     http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.apache.bookkeeper.tests.backwardcompat
+
+import com.github.dockerjava.api.DockerClient
+import org.apache.bookkeeper.tests.integration.utils.BookKeeperClusterUtils
+import org.apache.bookkeeper.tests.integration.utils.MavenClassLoader
+import org.jboss.arquillian.test.api.ArquillianResource
+import org.junit.Assert
+import org.junit.Before
+
+abstract class AbstractClientFencingTest {
+    protected static byte[] PASSWD = "foobar".getBytes()
+
+    @ArquillianResource
+    protected DockerClient docker
+
+    protected String currentVersion = BookKeeperClusterUtils.CURRENT_VERSION
+
+    @Before
+    public void before() throws Exception {
+        Assert.assertTrue(BookKeeperClusterUtils.stopAllBookies(docker))
+        // First test to run, formats metadata and bookies
+        if (BookKeeperClusterUtils.metadataFormatIfNeeded(docker, currentVersion)) {
+            BookKeeperClusterUtils.formatAllBookies(docker, currentVersion)
+        }
+        // If already started, this has no effect
+        Assert.assertTrue(BookKeeperClusterUtils.startAllBookiesWithVersion(docker, currentVersion))
+    }
+
+    protected void testFencingOldClient(String oldClientVersion, String fencingVersion) {
+        String zookeeper = BookKeeperClusterUtils.zookeeperConnectString(docker)
+
+        def oldCL = MavenClassLoader.forBookKeeperVersion(oldClientVersion)
+        def oldBK = oldCL.newBookKeeper(zookeeper)
+        def fencingCL = MavenClassLoader.forBookKeeperVersion(fencingVersion)
+        def fencingBK = fencingCL.newBookKeeper(zookeeper)
+
+        try {
+            def numEntries = 5
+            def ledger0 = oldBK.createLedger(3, 2,
+                    oldCL.digestType("CRC32"),
+                    PASSWD)
+            for (int i = 0; i < numEntries; i++) {
+                ledger0.addEntry(("foobar" + i).getBytes())
+            }
+            ledger0.close()
+
+
+            def ledger1 = fencingBK.openLedger(ledger0.getId(), fencingCL.digestType("CRC32"), PASSWD)
+
+            // cannot write any more
+            try {
+                ledger0.addEntry("shouldn't work".getBytes())
+                Assert.fail("Shouldn't have been able to add any more")
+            } catch (Exception e) {
+                Assert.assertEquals(e.getClass().getName(),
+                        "org.apache.bookkeeper.client.BKException\$BKLedgerClosedException")
+            }
+
+            // should be able to open it and read it back
+            def ledger2 = oldBK.openLedger(ledger0.getId(), oldCL.digestType("CRC32"), PASSWD)
+            def entries = ledger2.readEntries(0, ledger2.getLastAddConfirmed())
+            Assert.assertEquals(numEntries, ledger2.getLastAddConfirmed() + 1 /* counts from 0 */)
+            int j = 0
+            while (entries.hasMoreElements()) {
+                def e = entries.nextElement()
+                Assert.assertEquals(new String(e.getEntry()), "foobar"+ j)
+                j++
+            }
+            ledger2.close()
+        } catch(Throwable tt) {
+            throw new RuntimeException("Got exception with oldClientVersion: " + oldClientVersion + " and fencingVersion: " + fencingVersion, tt);
+        } finally {
+            oldBK.close()
+            oldCL.close()
+
+            fencingBK.close()
+            fencingCL.close()
+        }
+    }
+}

tests/backward-compat/current-server-old-clients/src/test/groovy/org/apache/bookkeeper/tests/backwardcompat/TestCompatNewClientFencesOldClient.groovy

@@ -0,0 +1,37 @@
+/*
+* Licensed to the Apache Software Foundation (ASF) under one
+* or more contributor license agreements.  See the NOTICE file
+* distributed with this work for additional information
+* regarding copyright ownership.  The ASF licenses this file
+* to you under the Apache License, Version 2.0 (the
+* "License"); you may not use this file except in compliance
+* with the License.  You may obtain a copy of the License at
+*
+*     http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.apache.bookkeeper.tests.backwardcompat
+
+import org.apache.bookkeeper.tests.integration.utils.BookKeeperClusterUtils
+import org.apache.bookkeeper.tests.integration.utils.ThreadReaper
+import org.jboss.arquillian.junit.Arquillian
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(Arquillian.class)
+class TestCompatNewClientFencesOldClient extends AbstractClientFencingTest {
+
+    @Test
+    public void testNewClientFencesOldClient() throws Exception {
+        BookKeeperClusterUtils.OLD_CLIENT_VERSIONS.each{
+            def version = it
+            ThreadReaper.runWithReaper({ testFencingOldClient(version, currentVersion) })
+        }
+    }
+
+}

tests/backward-compat/current-server-old-clients/src/test/groovy/org/apache/bookkeeper/tests/backwardcompat/TestCompatOldClientFencesOldClient.groovy

@@ -0,0 +1,40 @@
+/*
+* Licensed to the Apache Software Foundation (ASF) under one
+* or more contributor license agreements.  See the NOTICE file
+* distributed with this work for additional information
+* regarding copyright ownership.  The ASF licenses this file
+* to you under the Apache License, Version 2.0 (the
+* "License"); you may not use this file except in compliance
+* with the License.  You may obtain a copy of the License at
+*
+*     http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.apache.bookkeeper.tests.backwardcompat
+
+import com.github.dockerjava.api.DockerClient
+import org.apache.bookkeeper.tests.integration.utils.BookKeeperClusterUtils
+import org.apache.bookkeeper.tests.integration.utils.ThreadReaper
+import org.jboss.arquillian.junit.Arquillian
+import org.jboss.arquillian.test.api.ArquillianResource
+import org.junit.Assert
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(Arquillian.class)
+class TestCompatOldClientFencesOldClient extends AbstractClientFencingTest {
+
+    @Test
+    public void testOldClientFencesOldClient() throws Exception {
+        BookKeeperClusterUtils.OLD_CLIENT_VERSIONS.each{
+            def version = it
+            ThreadReaper.runWithReaper({ testFencingOldClient(version, version) })
+        }
+    }
+}