Multi-Cluster support for yamsql & support not-registering the driver with FRL#4042
Merged
ScottDugas merged 25 commits intoFoundationDB:mainfrom Apr 22, 2026
Merged
Multi-Cluster support for yamsql & support not-registering the driver with FRL#4042ScottDugas merged 25 commits intoFoundationDB:mainfrom
ScottDugas merged 25 commits intoFoundationDB:mainfrom
Conversation
It now will run all the test types, but non-0 clusters always go to the embedded or jdbc connection
ScottDugas
added
enhancement
This breaks backwards compatibility, but I don't think this is really intended to be a public interface
ScottDugas
added
the
Run mixed-mode
ohadzeliger
reviewed
Apr 8, 2026
| * @throws SQLException if the index is out of range | ||
| */ | ||
| @Nonnull | ||
| public Entry<T> get(int clusterIndex) throws SQLException { |
Contributor
There was a problem hiding this comment.
Would it make it easier to track if cluster would be identified by its file name rather than an index? Is there a method to the naming of the actual cluster files that would make tests easier to write when they actually are implemented?
Collaborator
Author
There was a problem hiding this comment.
File names are developer/environment dependent. We could give them names via the framework, but that would probably just be names like cluster1, which doesn't really improve the situation over using 1.
The idea here is primarily to support different identical clusters, so the key is not that 0 means something, just that it is consistent within the file so that you can control whether you are accessing the same or different clusters.
| public static List<String> allClusterFilesInRandomOrder() { | ||
| final List<String> randomized = new ArrayList<>(clusterFiles); | ||
| Collections.shuffle(randomized); | ||
| return List.copyOf(randomized); |
Contributor
There was a problem hiding this comment.
Is the copy necessary in this case? Can also return immutableList()
| @Nonnull | ||
| private Clusters<FRL> clusters = Clusters.empty(); | ||
|
|
||
| public EmbeddedConfig(@Nullable final String clusterFile) { |
Contributor
There was a problem hiding this comment.
Given the current setup, can clusterFile really be null?
Fixing names, javadoc, nullability annotations, removing unused code, using identical super method.
This ends up being slightly less useful than maybe desired, because most of the things that Clusters holds are production classes, and I don't want to add a new interface there (and we're not writing swift code)
ohadzeliger
approved these changes
Apr 10, 2026
g31pranjal
requested changes
Apr 16, 2026
I also cleaned up a couple of the yaml test tests that were more complicated than they needed to be.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The primary purpose here is to support multi-cluster testing to yaml tests.
In order to support this, a new option was added to FRL to not-register the driver so that you can create multiple different FRL instances, pointing to different clusters.
This is part of the work for: #3823