From d1db0d40135ca4859183824778335c6230628ac6 Mon Sep 17 00:00:00 2001 From: Scott Fines Date: Tue, 9 Mar 2021 14:14:54 -0600 Subject: [PATCH] Making it possible to run a multiple-cluster Java integration test --- bindings/java/CMakeLists.txt | 31 ++++--- bindings/java/src/README.md | 17 +++- .../BasicMultiClientIntegrationTest.java | 69 ++++++++++++++++ .../com/apple/foundationdb/DirectoryTest.java | 2 - .../apple/foundationdb/MultiClientHelper.java | 82 +++++++++++++++++++ bindings/java/src/tests.cmake | 2 + cmake/AddFdbTest.cmake | 33 +++++++- tests/TestRunner/tmp_cluster.py | 9 +- tests/TestRunner/tmp_multi_cluster.py | 75 +++++++++++++++++ 9 files changed, 302 insertions(+), 18 deletions(-) create mode 100644 bindings/java/src/integration/com/apple/foundationdb/BasicMultiClientIntegrationTest.java create mode 100644 bindings/java/src/integration/com/apple/foundationdb/MultiClientHelper.java create mode 100755 tests/TestRunner/tmp_multi_cluster.py diff --git a/bindings/java/CMakeLists.txt b/bindings/java/CMakeLists.txt index 2da8639b8d..bb9bc6359e 100644 --- a/bindings/java/CMakeLists.txt +++ b/bindings/java/CMakeLists.txt @@ -301,7 +301,7 @@ if(NOT OPEN_FOR_IDE) if(RUN_JUNIT_TESTS) # Sets up the JUnit testing structure to run through ctest # - # To add a new junit test, add the class to the JAVA_JUNIT_TESTS variable in `src/tests.cmake`. Note that if you run a Suite, + # To add a new junit test, add the class to the JAVA_JUNIT_TESTS variable in `src/tests.cmake`. Note that if you run a Suite, # ctest will NOT display underlying details of the suite itself, so it's best to avoid junit suites in general. Also, # if you need a different runner other than JUnitCore, you'll have to modify this so be aware. # @@ -309,8 +309,8 @@ if(NOT OPEN_FOR_IDE) # # ctest . # - # from the ${BUILD_DIR}/bindings/java subdirectory. - # + # from the ${BUILD_DIR}/bindings/java subdirectory. + # # Note: if you are running from ${BUILD_DIR}, additional tests of the native logic will be run. To avoid these, use # # ctest . -R java-unit @@ -318,15 +318,15 @@ if(NOT OPEN_FOR_IDE) # ctest has lots of flexible command options, so be sure to refer to its documentation if you want to do something specific(documentation # can be found at https://cmake.org/cmake/help/v3.19/manual/ctest.1.html) - add_jar(fdb-junit SOURCES ${JAVA_JUNIT_TESTS} ${JUNIT_RESOURCES} INCLUDE_JARS fdb-java - ${CMAKE_BINARY_DIR}/packages/junit-jupiter-api-5.7.1.jar + add_jar(fdb-junit SOURCES ${JAVA_JUNIT_TESTS} ${JUNIT_RESOURCES} INCLUDE_JARS fdb-java + ${CMAKE_BINARY_DIR}/packages/junit-jupiter-api-5.7.1.jar ${CMAKE_BINARY_DIR}/packages/junit-jupiter-engine-5.7.1.jar ${CMAKE_BINARY_DIR}/packages/junit-jupiter-params-5.7.1.jar ${CMAKE_BINARY_DIR}/packages/opentest4j-1.2.0.jar ${CMAKE_BINARY_DIR}/packages/apiguardian-api-1.1.1.jar ) get_property(junit_jar_path TARGET fdb-junit PROPERTY JAR_FILE) - + add_test(NAME java-unit COMMAND ${Java_JAVA_EXECUTABLE} -classpath "${target_jar}:${junit_jar_path}:${JUNIT_CLASSPATH}" @@ -339,12 +339,12 @@ if(NOT OPEN_FOR_IDE) if(RUN_JAVA_INTEGRATION_TESTS) # Set up the integration tests. These tests generally require a running database server to function properly. Most tests # should be written such that they can be run in parallel with other integration tests (e.g. try to use a unique key range for each test - # whenever possible), because it's a reasonable assumption that a single server will be shared among multiple tests, and might do so + # whenever possible), because it's a reasonable assumption that a single server will be shared among multiple tests, and might do so # concurrently. # # Integration tests are run through ctest the same way as unit tests, but their label is prefixed with the entry 'integration-'. - # Note that most java integration tests will fail if they can't quickly connect to a running FDB instance(depending on how the test is written, anyway). - # However, if you want to explicitly skip them, you can run + # Note that most java integration tests will fail if they can't quickly connect to a running FDB instance(depending on how the test is written, anyway). + # However, if you want to explicitly skip them, you can run # # `ctest -E integration` # @@ -361,8 +361,8 @@ if(NOT OPEN_FOR_IDE) # empty, consider generating a random prefix for the keys you write, use # the directory layer with a unique path, etc.) # - add_jar(fdb-integration SOURCES ${JAVA_INTEGRATION_TESTS} ${JAVA_INTEGRATION_RESOURCES} INCLUDE_JARS fdb-java - ${CMAKE_BINARY_DIR}/packages/junit-jupiter-api-5.7.1.jar + add_jar(fdb-integration SOURCES ${JAVA_INTEGRATION_TESTS} ${JAVA_INTEGRATION_RESOURCES} INCLUDE_JARS fdb-java + ${CMAKE_BINARY_DIR}/packages/junit-jupiter-api-5.7.1.jar ${CMAKE_BINARY_DIR}/packages/junit-jupiter-engine-5.7.1.jar ${CMAKE_BINARY_DIR}/packages/junit-jupiter-params-5.7.1.jar ${CMAKE_BINARY_DIR}/packages/opentest4j-1.2.0.jar @@ -375,7 +375,14 @@ if(NOT OPEN_FOR_IDE) COMMAND ${Java_JAVA_EXECUTABLE} -classpath "${target_jar}:${integration_jar_path}:${JUNIT_CLASSPATH}" -Djava.library.path=${CMAKE_BINARY_DIR}/lib - org.junit.platform.console.ConsoleLauncher "--details=summary" "--class-path=${integration_jar_path}" "--scan-classpath" "--disable-banner" + org.junit.platform.console.ConsoleLauncher "--details=summary" "--class-path=${integration_jar_path}" "--scan-classpath" "--disable-banner" "-T MultiClient" + ) + + add_multi_fdbclient_test(NAME java-multi-integration + COMMAND ${Java_JAVA_EXECUTABLE} + -classpath "${target_jar}:${integration_jar_path}:${JUNIT_CLASSPATH}" + -Djava.library.path=${CMAKE_BINARY_DIR}/lib + org.junit.platform.console.ConsoleLauncher "--details=summary" "--class-path=${integration_jar_path}" "--scan-classpath" "--disable-banner" "-t MultiClient" ) endif() diff --git a/bindings/java/src/README.md b/bindings/java/src/README.md index 6fedba6368..6bd377b85a 100644 --- a/bindings/java/src/README.md +++ b/bindings/java/src/README.md @@ -22,4 +22,19 @@ To skip integration tests, execute `ctest -E integration` from `${BUILD_DIR}/bin To run _only_ integration tests, run `ctest -R integration` from `${BUILD_DIR}/bindings/java`. There are lots of other useful `ctest` commands, which we don't need to get into here. For more information, -see the [https://cmake.org/cmake/help/v3.19/manual/ctest.1.html](ctest documentation). \ No newline at end of file +see the [https://cmake.org/cmake/help/v3.19/manual/ctest.1.html](ctest documentation). + +### Multi-Client tests +Multi-Client tests are integration tests that can only be executed when multiple clusters are running. To write a multi-client +test, do the following: + +1. Tag all tests that require multiple clients with `@Tag("MultiClient")` +2. Ensure that your tests have the `MultiClientHelper` extension present, and Registered as an extension +3. Ensure that your test class is in the the JAVA_INTEGRATION_TESTS list in `test.cmake` + +( see `BasicMultiClientIntegrationTest` for a good reference example) + +It is important to note that it requires significant time to start and stop 3 separate clusters; if the underying test takes a long time to run, +ctest will time out and kill the test. When that happens, there is no guarantee that the FDB clusters will be properly stopped! It is thus +in your best interest to ensure that all tests run in a relatively small amount of time, or have a longer timeout attached. + diff --git a/bindings/java/src/integration/com/apple/foundationdb/BasicMultiClientIntegrationTest.java b/bindings/java/src/integration/com/apple/foundationdb/BasicMultiClientIntegrationTest.java new file mode 100644 index 0000000000..2b02a3656f --- /dev/null +++ b/bindings/java/src/integration/com/apple/foundationdb/BasicMultiClientIntegrationTest.java @@ -0,0 +1,69 @@ +/* + * BasicMultiClientIntegrationTest + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2018 Apple Inc. and the FoundationDB project authors + * + * Licensed 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 com.apple.foundationdb; + +import java.util.Collection; +import java.util.Random; + +import com.apple.foundationdb.tuple.Tuple; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +/** + * Simple class to test multi-client logic. + * + * Note that all Multi-client-only tests _must_ be tagged with "MultiClient", which will ensure that they are excluded + * from non-multi-threaded tests. + */ +public class BasicMultiClientIntegrationTest { + @RegisterExtension public static final MultiClientHelper clientHelper = new MultiClientHelper(); + + @Test + @Tag("MultiClient") + void testMultiClientWritesAndReadsData() throws Exception { + FDB fdb = FDB.selectAPIVersion(630); + fdb.options().setKnob("min_trace_severity=5"); + + Collection dbs = clientHelper.openDatabases(fdb); // the clientHelper will close the databases for us + System.out.print("Starting tests."); + Random rand = new Random(); + for (int counter = 0; counter < 25; ++counter) { + for (Database db : dbs) { + String key = Integer.toString(rand.nextInt(100000000)); + String val = Integer.toString(rand.nextInt(100000000)); + + db.run(tr -> { + tr.set(Tuple.from(key).pack(), Tuple.from(val).pack()); + return null; + }); + + String fetchedVal = db.run(tr -> { + byte[] result = tr.get(Tuple.from(key).pack()).join(); + return Tuple.fromBytes(result).getString(0); + }); + Assertions.assertEquals(val, fetchedVal, "Wrong result!"); + } + Thread.sleep(200); + } + } +} diff --git a/bindings/java/src/integration/com/apple/foundationdb/DirectoryTest.java b/bindings/java/src/integration/com/apple/foundationdb/DirectoryTest.java index ddddd20ad1..519405e0f4 100644 --- a/bindings/java/src/integration/com/apple/foundationdb/DirectoryTest.java +++ b/bindings/java/src/integration/com/apple/foundationdb/DirectoryTest.java @@ -19,8 +19,6 @@ */ package com.apple.foundationdb; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.List; diff --git a/bindings/java/src/integration/com/apple/foundationdb/MultiClientHelper.java b/bindings/java/src/integration/com/apple/foundationdb/MultiClientHelper.java new file mode 100644 index 0000000000..671163955f --- /dev/null +++ b/bindings/java/src/integration/com/apple/foundationdb/MultiClientHelper.java @@ -0,0 +1,82 @@ +/* + * MultiClientHelper.java + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2018 Apple Inc. and the FoundationDB project authors + * + * Licensed 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 com.apple.foundationdb; + +import java.util.ArrayList; +import java.util.Collection; +import org.junit.jupiter.api.extension.AfterEachCallback; +import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.ExtensionContext; + +/** + * Callback to help define a multi-client scenario and ensure that + * the clients can be configured properly. + */ +public class MultiClientHelper implements BeforeAllCallback,AfterEachCallback{ + private String[] clusterFiles; + private Collection openDatabases; + + public static String[] readClusterFromEnv() { + /* + * Reads the cluster file lists from the ENV variable + * FDB_CLUSTERS. + */ + String clusterFilesProp = System.getenv("FDB_CLUSTERS"); + if (clusterFilesProp == null) { + throw new IllegalStateException("Missing FDB cluster connection file names"); + } + + return clusterFilesProp.split(";"); + } + + Collection openDatabases(FDB fdb){ + if(openDatabases!=null){ + return openDatabases; + } + if(clusterFiles==null){ + clusterFiles = readClusterFromEnv(); + } + Collection dbs = new ArrayList(); + for (String arg : clusterFiles) { + System.out.printf("Opening Cluster: %s\n", arg); + dbs.add(fdb.open(arg)); + } + + this.openDatabases = dbs; + return dbs; + } + + @Override + public void beforeAll(ExtensionContext arg0) throws Exception { + clusterFiles = readClusterFromEnv(); + } + + @Override + public void afterEach(ExtensionContext arg0) throws Exception { + //close any databases that have been opened + if(openDatabases!=null){ + for(Database db : openDatabases){ + db.close(); + } + } + openDatabases = null; + } + +} diff --git a/bindings/java/src/tests.cmake b/bindings/java/src/tests.cmake index 8bed62ecb8..bf3131eeb3 100644 --- a/bindings/java/src/tests.cmake +++ b/bindings/java/src/tests.cmake @@ -48,12 +48,14 @@ set(JUNIT_RESOURCES set(JAVA_INTEGRATION_TESTS src/integration/com/apple/foundationdb/DirectoryTest.java src/integration/com/apple/foundationdb/RangeQueryIntegrationTest.java + src/integration/com/apple/foundationdb/BasicMultiClientIntegrationTest.java ) # Resources that are used in integration testing, but are not explicitly test files (JUnit rules, # utility classes, and so forth) set(JAVA_INTEGRATION_RESOURCES src/integration/com/apple/foundationdb/RequiresDatabase.java + src/integration/com/apple/foundationdb/MultiClientHelper.java ) diff --git a/cmake/AddFdbTest.cmake b/cmake/AddFdbTest.cmake index 7e0502c52e..3824edc9b2 100644 --- a/cmake/AddFdbTest.cmake +++ b/cmake/AddFdbTest.cmake @@ -378,6 +378,7 @@ function(package_bindingtester) add_dependencies(bindingtester copy_bindingtester_binaries) endfunction() +# Creates a single cluster before running the specified command (usually a ctest test) function(add_fdbclient_test) set(options DISABLED ENABLED) set(oneValueArgs NAME) @@ -401,7 +402,37 @@ function(add_fdbclient_test) --build-dir ${CMAKE_BINARY_DIR} -- ${T_COMMAND}) - set_tests_properties("${T_NAME}" PROPERTIES TIMEOUT 60) + set_tests_properties("${T_NAME}" PROPERTIES TIMEOUT 60) +endfunction() + +# Creates 3 distinct clusters before running the specified command. +# This is useful for testing features that require multiple clusters (like the +# multi-cluster FDB client) +function(add_multi_fdbclient_test) + set(options DISABLED ENABLED) + set(oneValueArgs NAME) + set(multiValueArgs COMMAND) + cmake_parse_arguments(T "${options}" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}") + if(OPEN_FOR_IDE) + return() + endif() + if(NOT T_ENABLED AND T_DISABLED) + return() + endif() + if(NOT T_NAME) + message(FATAL_ERROR "NAME is a required argument for add_multi_fdbclient_test") + endif() + if(NOT T_COMMAND) + message(FATAL_ERROR "COMMAND is a required argument for add_multi_fdbclient_test") + endif() + message(STATUS "Adding Client test ${T_NAME}") + add_test(NAME "${T_NAME}" + COMMAND ${CMAKE_SOURCE_DIR}/tests/TestRunner/tmp_multi_cluster.py + --build-dir ${CMAKE_BINARY_DIR} + --clusters 3 + -- + ${T_COMMAND}) + set_tests_properties("${T_NAME}" PROPERTIES TIMEOUT 60) endfunction() function(add_java_test) diff --git a/tests/TestRunner/tmp_cluster.py b/tests/TestRunner/tmp_cluster.py index c34fc6a85e..f8ae4ef813 100755 --- a/tests/TestRunner/tmp_cluster.py +++ b/tests/TestRunner/tmp_cluster.py @@ -11,7 +11,7 @@ from random import choice from pathlib import Path class TempCluster: - def __init__(self, build_dir: str): + def __init__(self, build_dir: str,port: str = None): self.build_dir = Path(build_dir).resolve() assert self.build_dir.exists(), "{} does not exist".format(build_dir) assert self.build_dir.is_dir(), "{} is not a directory".format(build_dir) @@ -22,7 +22,8 @@ class TempCluster: self.cluster = LocalCluster(tmp_dir, self.build_dir.joinpath('bin', 'fdbserver'), self.build_dir.joinpath('bin', 'fdbmonitor'), - self.build_dir.joinpath('bin', 'fdbcli')) + self.build_dir.joinpath('bin', 'fdbcli'), + port = port) self.log = self.cluster.log self.etc = self.cluster.etc self.data = self.cluster.data @@ -37,6 +38,10 @@ class TempCluster: self.cluster.__exit__(xc_type, exc_value, traceback) shutil.rmtree(self.tmp_dir) + def close(self): + self.cluster.__exit__(None,None,None) + shutil.rmtree(self.tmp_dir) + if __name__ == '__main__': parser = ArgumentParser(formatter_class=RawDescriptionHelpFormatter, diff --git a/tests/TestRunner/tmp_multi_cluster.py b/tests/TestRunner/tmp_multi_cluster.py new file mode 100755 index 0000000000..2edcef29de --- /dev/null +++ b/tests/TestRunner/tmp_multi_cluster.py @@ -0,0 +1,75 @@ +#!/usr/bin/env python3 + +# +# tmp_multi_cluster.py +# +# This source file is part of the FoundationDB open source project +# +# Copyright 2013-2018 Apple Inc. and the FoundationDB project authors +# +# Licensed 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. +# + +import os +import subprocess +import sys +import shutil +from pathlib import Path +from argparse import ArgumentParser, RawDescriptionHelpFormatter +from tmp_cluster import TempCluster + + +if __name__ == '__main__': + parser = ArgumentParser(formatter_class=RawDescriptionHelpFormatter,description=""" + This script automatically configures N temporary local clusters on the machine and then + calls a command while these clusters are running. As soon as the command returns, all + configured clusters are killed and all generated data is deleted. + + The purpose of this is to support testing a set of integration tests using multiple clusters + (i.e. using the Multi-threaded client). + """) + parser.add_argument('--build-dir','-b',metavar='BUILD_DIRECTORY',help='FDB build director',required=True) + parser.add_argument('--clusters','-c',metavar='NUM_CLUSTERS',type=int,help='The number of clusters to run',required=True) + parser.add_argument('cmd', metavar='COMMAND',nargs='+',help='The command to run') + args = parser.parse_args() + errcode = 1 + + #spawn all the clusters + base_dir = args.build_dir + num_clusters = args.clusters + + build_dir=Path(base_dir) + bin_dir=build_dir.joinpath('bin') + + clusters = [] + for c in range(1,num_clusters+1): + # now start the cluster up + local_c = TempCluster(args.build_dir, port="{}501".format(c)) + + local_c.__enter__() + clusters.append(local_c) + + # all clusters should be running now, so run the subcommand + # TODO (bfines): pass through the proper ENV commands so that the client can find everything + cluster_paths = ';'.join([str(cluster.etc.joinpath('fdb.cluster')) for cluster in clusters]) + print(cluster_paths) + env = dict(**os.environ) + env['FDB_CLUSTERS'] = env.get('FDB_CLUSTERS',cluster_paths) + errcode = subprocess.run(args.cmd,stdout=sys.stdout,stderr=sys.stderr,env=env).returncode + + # shutdown all the running clusters + for tc in clusters: + tc.close() + + sys.exit(errcode) + \ No newline at end of file