-
Notifications
You must be signed in to change notification settings - Fork 112
Add support for testing against multiple FDB clusters #3575
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ScottDugas
wants to merge
35
commits into
FoundationDB:main
Choose a base branch
from
ScottDugas:multiple-fdb-clusters
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
99c8a89
Add support for connecting to multiple cluster files in tests
ScottDugas 43aa388
Add support for testing specifically against multiple clusters
ScottDugas c26e200
Ensure FDB gets opened even if there are no cluster files
ScottDugas 672b305
Have fdb-extensions get cluster info from yaml too
ScottDugas f4aea84
Fix handling behavior when there is no cluster file specified
ScottDugas 11c900a
Throw RecordCoreException in FDBDatabaseExtepsion setup
ScottDugas d15b81e
Fix up exception conversion
ScottDugas d31a46d
Start two fdb clusters in CI
ScottDugas e058377
Add snakeyaml dependency to projects depending on core tests
ScottDugas 8e84a0e
Fix sqlline test
ScottDugas 71db28c
Use dbExtension.getDatabase in FDBRecordContextTest
ScottDugas 68fa77a
Create database with cluster file in ResolverMappingReplicatorTest.te…
ScottDugas ad284a6
Fixup RankIndexTest.writeOnlyRankQuery
ScottDugas f418350
Try creating an FDBTestClusterConfig in testFixtures
ScottDugas 2b9af6d
Don't use fixtures yet
ScottDugas 66c5f5f
Update some tests to get random cluster files from FDBTestEnvironment
ScottDugas 9cb9f2c
Update remaining tests to not use `null` clusterFile
ScottDugas 134938e
Fix checkstyle/javadoc checks
ScottDugas 64deb2b
Add option to start RelationalServer with a clusterFile & use that
ScottDugas e48f0f3
LocatableResolverTest needs to use the same clusterFile in tests that…
ScottDugas 74b886e
Fix last couple tests that wipe FDB
ScottDugas 6a953b0
Fix JDBC tests that depend on creating in process server
ScottDugas d1c1ee1
Fixup InProcessRelationalServer to use FDBTestEnvironment
ScottDugas fa20608
Update yaml-tests to use FDBTestEnvironment
ScottDugas 44037f2
Have YamlConnection provide cluster file so we can apply direct opera…
ScottDugas dae9a82
ExternalServerTest needs to provide a clusterFile
ScottDugas ffec130
Fixup comments in FDBTestEnvironment
ScottDugas 4617fc7
Change TestDatabaseExtension to pick a random database
ScottDugas 83f2657
Cleanup some cases where tests were using multiple clusters unintenti…
ScottDugas c9eeaa4
Have FDBDatabaseExtension use a single cluster for the test
ScottDugas 1e5552f
Always provide a cluster file, and reset in afterEach
ScottDugas 244c5f0
Properly handle null clusterFile
ScottDugas 0b63c36
Merge branch 'main' into multiple-fdb-clusters
ScottDugas e4d4631
Fixup post merge
ScottDugas 252f879
Fixup dependencies to use testFixtures.
ScottDugas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
86 changes: 86 additions & 0 deletions
86
fdb-extensions/src/testFixtures/java/com/apple/foundationdb/test/FDBTestEnvironment.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
/* | ||
* FDBTestEnvironment.java | ||
* | ||
* This source file is part of the FoundationDB open source project | ||
* | ||
* Copyright 2015-2025 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.test; | ||
|
||
import org.assertj.core.api.Assumptions; | ||
import org.yaml.snakeyaml.Yaml; | ||
|
||
import javax.annotation.Nullable; | ||
import java.io.FileInputStream; | ||
import java.io.IOException; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.concurrent.ThreadLocalRandom; | ||
|
||
/** | ||
* Utility class for accessing {@code fdb-environment.yaml}, so that tests can know about fdb clusters | ||
* that are available. | ||
*/ | ||
public final class FDBTestEnvironment { | ||
private static final List<String> clusterFiles; | ||
|
||
static { | ||
final String fdbEnvironment = System.getenv("FDB_ENVIRONMENT_YAML"); | ||
|
||
if (fdbEnvironment != null && !fdbEnvironment.isEmpty()) { | ||
clusterFiles = List.copyOf(parseFDBEnvironmentYaml(fdbEnvironment)); | ||
} else { | ||
clusterFiles = Collections.singletonList(null); // List.of does not allow null | ||
} | ||
} | ||
|
||
@SuppressWarnings("unchecked") | ||
private static List<String> parseFDBEnvironmentYaml(final String fdbEnvironment) { | ||
Yaml yaml = new Yaml(); | ||
try (FileInputStream yamlInput = new FileInputStream(fdbEnvironment)) { | ||
Object fdbConfig = yaml.load(yamlInput); | ||
return (List<String>)((Map<?, ?>)fdbConfig).get("clusterFiles"); | ||
} catch (IOException e) { | ||
throw new IllegalStateException("Could not read fdb-environment.yaml", e); | ||
} catch (ClassCastException e) { | ||
throw new IllegalStateException("Could not parse fdb environment file " + fdbEnvironment, e); | ||
} | ||
} | ||
|
||
@Nullable | ||
public static String getClusterFile(int i) { | ||
return clusterFiles.get(i); | ||
} | ||
|
||
public static List<String> allClusterFiles() { | ||
return clusterFiles; | ||
} | ||
|
||
public static String randomClusterFile() { | ||
return clusterFiles.get(ThreadLocalRandom.current().nextInt(clusterFiles.size())); | ||
} | ||
|
||
/** | ||
* Marks the current test as skipped if there are not the desired number of clusters available to test against. | ||
* @param desiredCount the number of clusters to test against | ||
*/ | ||
public static void assumeClusterCount(final int desiredCount) { | ||
if (desiredCount > 1) { | ||
Assumptions.assumeThat(clusterFiles.size()).as("Cluster file count").isGreaterThanOrEqualTo(desiredCount); | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ | |
import com.apple.foundationdb.record.RecordCoreException; | ||
import com.apple.foundationdb.record.TestHelpers; | ||
import com.apple.foundationdb.record.test.FDBDatabaseExtension; | ||
import com.apple.foundationdb.test.FDBTestEnvironment; | ||
import com.apple.test.Tags; | ||
import org.junit.jupiter.api.Tag; | ||
import org.junit.jupiter.api.Test; | ||
|
@@ -57,7 +58,7 @@ void testBlockingInAsyncException() { | |
// Make sure that we aren't holding on to previously created databases | ||
factory.clear(); | ||
|
||
FDBDatabase database = factory.getDatabase(); | ||
FDBDatabase database = factory.getDatabase(FDBTestEnvironment.randomClusterFile()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test could probably just use |
||
assertEquals(BlockingInAsyncDetection.IGNORE_COMPLETE_EXCEPTION_BLOCKING, database.getBlockingInAsyncDetection()); | ||
assertThrows(BlockingInAsyncException.class, () -> callAsyncBlocking(database)); | ||
} | ||
|
@@ -68,7 +69,7 @@ void testBlockingInAsyncWarning() { | |
factory.setBlockingInAsyncDetection(BlockingInAsyncDetection.IGNORE_COMPLETE_WARN_BLOCKING); | ||
factory.clear(); | ||
|
||
FDBDatabase database = factory.getDatabase(); | ||
FDBDatabase database = factory.getDatabase(FDBTestEnvironment.randomClusterFile()); | ||
TestHelpers.assertLogs(FDBDatabase.class, FDBDatabase.BLOCKING_IN_ASYNC_CONTEXT_MESSAGE, | ||
() -> { | ||
callAsyncBlocking(database, true); | ||
|
@@ -82,7 +83,7 @@ void testCompletedBlockingInAsyncWarning() { | |
factory.setBlockingInAsyncDetection(BlockingInAsyncDetection.WARN_COMPLETE_EXCEPTION_BLOCKING); | ||
factory.clear(); | ||
|
||
FDBDatabase database = factory.getDatabase(); | ||
FDBDatabase database = factory.getDatabase(FDBTestEnvironment.randomClusterFile()); | ||
TestHelpers.assertLogs(FDBDatabase.class, FDBDatabase.BLOCKING_IN_ASYNC_CONTEXT_MESSAGE, | ||
() -> database.asyncToSync(new FDBStoreTimer(), FDBStoreTimer.Waits.WAIT_ERROR_CHECK, | ||
CompletableFuture.supplyAsync(() -> | ||
|
@@ -95,7 +96,7 @@ void testBlockingCreatingAsyncDetection() { | |
factory.setBlockingInAsyncDetection(BlockingInAsyncDetection.WARN_COMPLETE_EXCEPTION_BLOCKING); | ||
factory.clear(); | ||
|
||
FDBDatabase database = factory.getDatabase(); | ||
FDBDatabase database = factory.getDatabase(FDBTestEnvironment.randomClusterFile()); | ||
TestHelpers.assertLogs(FDBDatabase.class, FDBDatabase.BLOCKING_RETURNING_ASYNC_MESSAGE, | ||
() -> returnAnAsync(database, MoreAsyncUtil.delayedFuture(200L, TimeUnit.MILLISECONDS, database.getScheduledExecutor()))); | ||
} | ||
|
@@ -106,7 +107,7 @@ void testCompletedBlockingCreatingAsyncDetection() { | |
factory.setBlockingInAsyncDetection(BlockingInAsyncDetection.WARN_COMPLETE_EXCEPTION_BLOCKING); | ||
factory.clear(); | ||
|
||
FDBDatabase database = factory.getDatabase(); | ||
FDBDatabase database = factory.getDatabase(FDBTestEnvironment.randomClusterFile()); | ||
TestHelpers.assertDidNotLog(FDBDatabase.class, FDBDatabase.BLOCKING_RETURNING_ASYNC_MESSAGE, | ||
() -> returnAnAsync(database, CompletableFuture.completedFuture(10L))); | ||
} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about putting this in
testFixtures
, but since other projects already depend on the tests configuration, doing so would require moving all the other classes into atestFixtures
, and I figured that would be better as a separate PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a change in main to move them into a test fixture, and then went to test publishing of
testFixtures
so thatyaml-tests
could be used by clients for their tests, but it looks like we can't publishtestFixtures
: e9db5c0#diff-9515853bb3a0c957c759e9c453a9e17941f4333751715e75d74efed681fadac0R60 I'm going to look into creating atest-utils
library....