Skip to content

Commit

Permalink
Resolves #1639: Support a configurable FDB API version (#1642)
Browse files Browse the repository at this point in the history
* Resolves #1639: Support a configurable FDB API version

This makes the FDB API version configurable through the `FDBDatabaseFactory`.

We'll need some better way of testing this, I think. I've tried running the tests at both the 6.3 and 7.1 API versions, and it passed, but this value cannot be changed after the client has been initiated, which makes this somewhat complex to test with our normal approach. We might be able to use a test-only implementation of `FDBDatabaseFactory` if we wanted to, say, test future work that had feature flags on the API version. Another approach might be to run the tests multiple times, once per API version.

Also added the ability to make the API version a gradle configuration property. Setting `-PapiVersion=N` will run the tests at API version `N`. We could hook this up later to run the tests multiple times at different API versions if we wish

This resolves #1639.
  • Loading branch information
alecgrieser committed May 6, 2022
1 parent 36768b7 commit d19f7b2
Show file tree
Hide file tree
Showing 15 changed files with 398 additions and 47 deletions.
3 changes: 3 additions & 0 deletions build.gradle
Expand Up @@ -68,6 +68,9 @@ allprojects {
}
project.tasks.withType(Test) {
systemProperty "file.encoding", "utf-8"
if (project.hasProperty("apiVersion")) {
systemProperty "com.apple.foundationdb.apiVersion", project.getProperty("apiVersion")
}
}

buildDir = ".out"
Expand Down
6 changes: 5 additions & 1 deletion docs/ReleaseNotes.md
Expand Up @@ -6,6 +6,10 @@ As the [versioning guide](Versioning.md) details, it cannot always be determined

## 3.2

### Features

This version of the Record Layer allows the FDB API version to be configured through the `FDBDatabaseFactory`. This means that while this version allows the client to be configured to use 7.1 features, it also supports connecting to 6.3 FDB clusters if the API version is set appropriately. Note that setting the API version does restrict the set of potential FDB server versions that can be connected to, so this configuration change should only be made if the FDB server has already been updated.

### Breaking Changes

The FoundationDB Java binding dependency has been updated to 7.1 with this release. This means that clients also need to update their main FDB C client to a 7.1 version. Adopters that still wish to connect to an FDB cluster running a 6.3 or 7.0 server version can do so by packaging additional FDB C clients at the appropriate version(s) using the [FDB multi-version client feature](https://apple.github.io/foundationdb/api-general.html#multi-version-client).
Expand All @@ -26,7 +30,7 @@ This release also updates downstream dependency versions. Most notably, the prot
* **Performance** Improvement 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Performance** Improvement 4 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Performance** Improvement 5 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Feature 1 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** The FDB API version can now be configured through the `FDBDatabaseFactory` [(Issue #1639)](https://github.com/FoundationDB/fdb-record-layer/issues/1639)
* **Feature** Feature 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Feature 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Feature 4 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
Expand Down
Expand Up @@ -26,9 +26,22 @@
* Base class that all tests touching FoundationDB should inherit from.
*/
public abstract class FDBTestBase {
private static final int MIN_API_VERSION = 630;
private static final int MAX_API_VERSION = 710;
private static final String API_VERSION_PROPERTY = "com.apple.foundationdb.apiVersion";

private static int getAPIVersion() {
int apiVersion = Integer.parseInt(System.getProperty(API_VERSION_PROPERTY, "630"));
if (apiVersion < MIN_API_VERSION || apiVersion > MAX_API_VERSION) {
throw new IllegalStateException(String.format("unsupported API version %d (must be between %d and %d)",
apiVersion, MIN_API_VERSION, MAX_API_VERSION));
}
return apiVersion;
}

@BeforeAll
public static void setupFDB() {
FDB fdb = FDB.selectAPIVersion(630);
FDB fdb = FDB.selectAPIVersion(getAPIVersion());
fdb.setUnclosedWarning(true);
}
}
4 changes: 2 additions & 2 deletions fdb-record-layer-core/fdb-record-layer-core.gradle
Expand Up @@ -81,8 +81,8 @@ test {

// Configure whether or not tests will validate that asyncToSync isn't being called in async
// context. See BlockingInAsyncDetection class for details on values.
systemProperties = [ "com.apple.foundationdb.record.blockingInAsyncDetection":
System.getenv('BLOCKING_DETECTION') ?: "IGNORE_COMPLETE_EXCEPTION_BLOCKING" ]
systemProperty "com.apple.foundationdb.record.blockingInAsyncDetection",
System.getenv('BLOCKING_DETECTION') ?: "IGNORE_COMPLETE_EXCEPTION_BLOCKING"

// We need the destructive tests to be available in the 'test' task, so that IntelliJ's gradle test runner can
// find them. However, we _only_ want to run them as part of the 'test' task if we're running from IntelliJ.
Expand Down
Expand Up @@ -165,6 +165,15 @@ public enum LogMessageKeys {
INDEXING_POLICY,
INDEXING_POLICY_DESIRED_ACTION,

// FDB client configuration
API_VERSION,
RUN_LOOP_PROFILING,
THREADS_PER_CLIENT_VERSION,
TRACE_DIRECTORY,
TRACE_FORMAT,
TRACE_LOG_GROUP,
UNCLOSED_WARNING,

// comparisons
COMPARISON_VALUE,
EXPECTED,
Expand Down
@@ -0,0 +1,123 @@
/*
* APIVersion.java
*
* This source file is part of the FoundationDB open source project
*
* Copyright 2015-2022 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.record.provider.foundationdb;

import com.apple.foundationdb.annotation.API;
import com.apple.foundationdb.record.RecordCoreArgumentException;

import javax.annotation.Nonnull;

/**
* An enum representing the different supported FDB API versions. The
* <a href="https://apple.github.io/foundationdb/api-general.html#api-versions">FDB API version</a>
* controls the minimum FDB server version that can be successfully connected to, as well as
* what features are supported by the FDB client. It is therefore important that in scenarios where
* there are FDB servers on different versions that the API version be configured to not exceed the
* lowest FDB server version. If the API version is set too high, the FDB client may hang when
* trying to connect to the server.
*
* <p>
* Because FDB API versions also control which features are guaranteed to be supported, there may
* be Record Layer features that are gated on specific FDB API versions. For that reason, if there
* is a new feature that relies on specific FDB server features being available, the client may
* need to be configured with the appropriate API version before that feature can be used.
* </p>
*
* <p>
* A note on Record Layer API stability: the elements of this enum are expected to shift as support
* for FDB versions are deprecated and removed. As a result, the elements of this enum will be added
* and removed as support for old FDB versions is dropped and new FDB versions is added. Setting the
* API version to a non-default value is also currently an experimental feature as additional testing
* and validation work to evaluate the library at various FDB API versions is developed.
* </p>
*
* @see FDBDatabaseFactory#setAPIVersion(APIVersion)
*/
@API(API.Status.EXPERIMENTAL)
public enum APIVersion {
/**
* API version corresponding to FoundationDB 6.3.
*/
API_VERSION_6_3(630),
/**
* API version corresponding to FoundationDB 7.0.
*/
API_VERSION_7_0(700),
/**
* API version corresponding to FoundationDB 7.1.
*/
API_VERSION_7_1(710),
;

private final int versionNumber;

APIVersion(int versionNumber) {
this.versionNumber = versionNumber;
}

/**
* Get the API version number. This is the value passed to FDB during client initialization.
* @return the version number associated with this version of the API
*/
public int getVersionNumber() {
return versionNumber;
}

/**
* Whether this API version is at least as new as another API version. This is
* useful for determining if a feature added at a certain API version is available.
*
* @param other the API version to compare against
* @return whether this API version is at least the provided API version
*/
public boolean isAtLeast(@Nonnull APIVersion other) {
return versionNumber >= other.getVersionNumber();
}

/**
* Get the default API version. Unless the adopter overrides this value by calling
* {@link FDBDatabaseFactory#setAPIVersion(APIVersion)}, this is the API version that will
* be used to initialize the FDB client connection.
*
* <p>
* This is currently {@link #API_VERSION_6_3}.
* </p>
*
* @return the default API version
*/
public static APIVersion getDefault() {
return API_VERSION_6_3;
}

/**
* Get the {@code APIVersion} enum from the version number.
* @param versionNumber the FDB API version number
* @return the corresponding enum value
*/
public static APIVersion fromVersionNumber(int versionNumber) {
for (APIVersion version : values()) {
if (version.getVersionNumber() == versionNumber) {
return version;
}
}
throw new RecordCoreArgumentException("api version not supported");
}
}
Expand Up @@ -153,7 +153,10 @@ public class FDBDatabase {
private String datacenterId;

@Nonnull
private FDBLocalityProvider localityProvider;
private final FDBLocalityProvider localityProvider;

@Nonnull
private final APIVersion apiVersion;

@Nonnull
private static ImmutablePair<Long, Long> initialVersionPair = new ImmutablePair<>(null, null);
Expand Down Expand Up @@ -184,6 +187,7 @@ public FDBDatabase(@Nonnull FDBDatabaseFactory factory, @Nullable String cluster
this.latencyInjector = factory.getLatencyInjector();
this.datacenterId = factory.getDatacenterId();
this.localityProvider = factory.getLocalityProvider();
this.apiVersion = factory.getAPIVersion();
}

/**
Expand Down Expand Up @@ -238,6 +242,23 @@ public synchronized FDBLocalityProvider getLocalityProvider() {
return localityProvider;
}

/**
* Get the FDB API version that this database was created with. This can be used by calling
* code to determine what methods are safe to call, and for compatibility code to know how to
* properly execute operations that may require differently depending on the API. This is an
* internal method that should only be called by internal modules, as higher level Record
* Layer APIs should shield adopters from needing to know this configuration value except
* possibly during application initialization.
*
* @return the API version of this database
* @see APIVersion
* @see FDBDatabaseFactory#setAPIVersion(APIVersion)
*/
@API(API.Status.INTERNAL)
public APIVersion getAPIVersion() {
return apiVersion;
}

public synchronized void setTrackLastSeenVersionOnRead(boolean trackLastSeenVersion) {
this.trackLastSeenVersionOnRead = trackLastSeenVersion;
}
Expand Down Expand Up @@ -1266,7 +1287,7 @@ private void logOrThrowBlockingInAsync(@Nonnull BlockingInAsyncDetection behavio
boolean isComplete,
@Nonnull StackTraceElement stackElement,
@Nonnull String title) {
final RuntimeException exception = new BlockingInAsyncException(title)
final RecordCoreException exception = new BlockingInAsyncException(title)
.addLogInfo(
LogMessageKeys.FUTURE_COMPLETED, isComplete,
LogMessageKeys.CALLING_CLASS, stackElement.getClassName(),
Expand All @@ -1276,12 +1297,9 @@ private void logOrThrowBlockingInAsync(@Nonnull BlockingInAsyncDetection behavio
if (!isComplete && behavior.throwExceptionOnBlocking()) {
throw exception;
} else {
LOGGER.warn(KeyValueLogMessage.of(title,
LogMessageKeys.FUTURE_COMPLETED, isComplete,
LogMessageKeys.CALLING_CLASS, stackElement.getClassName(),
LogMessageKeys.CALLING_METHOD, stackElement.getMethodName(),
LogMessageKeys.CALLING_LINE, stackElement.getLineNumber()),
exception);
KeyValueLogMessage logMessage = KeyValueLogMessage.build(title)
.addKeysAndValues(exception.getLogInfo());
LOGGER.warn(logMessage.toString(), exception);
}
}

Expand Down
Expand Up @@ -666,6 +666,42 @@ public TransactionListener getTransactionListener() {

public abstract Supplier<Boolean> getTransactionIsTracedSupplier();

/**
* Set the API version for this database factory. This value will be used to determine what FDB APIs are
* available.
*
* <p>
* Note that once an API version has been set, the client will not be able to talk to any FDB cluster
* that is running an older associated server version. It is therefore important that the adopter sets
* this value so that it does not exceed the smallest FDB server version that this client will be
* expected to connect to. However, if there are features that are only available on a subset of
* supported FDB API versions, the Record Layer will attempt to guard those features so that they are
* only used if the configured API version supports it.
* </p>
*
* <p>
* By default, this will be set to {@link APIVersion#getDefault()}. Note that setting the API version
* to a non-default value is currently experimental as additional testing and validation of various
* API versions is developed.
* </p>
*
* @param apiVersion the API version to use when initializing the FDB client
* @see APIVersion
*/
@API(API.Status.EXPERIMENTAL)
public abstract void setAPIVersion(@Nonnull APIVersion apiVersion);

/**
* Get the configured FDB API version. This is an internal method to indicate what value was configured
* on this database factory, as other Record Layer APIs should hide API-version compatibility code so
* that adopters do not need to worry about setting this value except possibly during client initialization.
*
* @return the configured FDB API version
* @see #setAPIVersion(APIVersion)
*/
@API(API.Status.INTERNAL)
public abstract APIVersion getAPIVersion();

@Nonnull
public abstract FDBDatabase getDatabase(@Nullable String clusterFile);

Expand Down
Expand Up @@ -27,6 +27,7 @@
import com.apple.foundationdb.annotation.SpotBugsSuppressWarnings;
import com.apple.foundationdb.record.RecordCoreException;
import com.apple.foundationdb.record.logging.KeyValueLogMessage;
import com.apple.foundationdb.record.logging.LogMessageKeys;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -41,7 +42,6 @@
public class FDBDatabaseFactoryImpl extends FDBDatabaseFactory {

private static final Logger LOGGER = LoggerFactory.getLogger(FDBDatabaseFactory.class);
private static final int API_VERSION = 630;

@Nonnull
private static final FDBDatabaseFactoryImpl INSTANCE = new FDBDatabaseFactoryImpl();
Expand Down Expand Up @@ -81,6 +81,8 @@ public class FDBDatabaseFactoryImpl extends FDBDatabaseFactory {
private String traceLogGroup = null;
@Nonnull
private FDBTraceFormat traceFormat = FDBTraceFormat.DEFAULT;
@Nonnull
private APIVersion apiVersion = APIVersion.getDefault();

private boolean runLoopProfilingEnabled = false;

Expand All @@ -98,10 +100,18 @@ public static FDBDatabaseFactoryImpl instance() {

protected synchronized FDB initFDB() {
if (!inited) {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug(KeyValueLogMessage.of("Starting FDB"));
if (LOGGER.isInfoEnabled()) {
LOGGER.info(KeyValueLogMessage.build("Staring FDB")
.addKeyAndValue(LogMessageKeys.API_VERSION, apiVersion.getVersionNumber())
.addKeyAndValue(LogMessageKeys.UNCLOSED_WARNING, unclosedWarning)
.addKeyAndValue(LogMessageKeys.TRACE_FORMAT, traceFormat)
.addKeyAndValue(LogMessageKeys.TRACE_DIRECTORY, traceDirectory)
.addKeyAndValue(LogMessageKeys.TRACE_LOG_GROUP, traceLogGroup)
.addKeyAndValue(LogMessageKeys.RUN_LOOP_PROFILING, runLoopProfilingEnabled)
.addKeyAndValue(LogMessageKeys.THREADS_PER_CLIENT_VERSION, threadsPerClientVersion)
.getMessageWithKeys());
}
fdb = FDB.selectAPIVersion(API_VERSION);
fdb = FDB.selectAPIVersion(apiVersion.getVersionNumber());
fdb.setUnclosedWarning(unclosedWarning);
setStaticOptions(fdb);
NetworkOptions options = fdb.options();
Expand Down Expand Up @@ -173,6 +183,22 @@ public void setTraceFormat(@Nonnull FDBTraceFormat traceFormat) {
this.traceFormat = traceFormat;
}

@Override
public synchronized void setAPIVersion(@Nonnull APIVersion apiVersion) {
if (this.apiVersion == apiVersion) {
return;
}
if (inited) {
throw new RecordCoreException("API version cannot be changed after client has already started");
}
this.apiVersion = apiVersion;
}

@Override
public synchronized APIVersion getAPIVersion() {
return apiVersion;
}

@Override
public synchronized void setRunLoopProfilingEnabled(boolean runLoopProfilingEnabled) {
if (inited) {
Expand Down

0 comments on commit d19f7b2

Please sign in to comment.