Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
178cd61
Initial pass at KeySpacePath.importData
ScottDugas Sep 5, 2025
938d240
Cleanup some of the tests for importing data
ScottDugas Sep 8, 2025
bee1806
Cleanup import tests
ScottDugas Sep 8, 2025
44d609b
A little more test cleanup
ScottDugas Sep 8, 2025
b810497
Respond to api change on main for DataInKeySpacePath
ScottDugas Oct 24, 2025
9132184
Extract helper for export+import
ScottDugas Oct 24, 2025
f63ae18
Change the import test to use 2 clusters if available
ScottDugas Oct 24, 2025
714faf3
Respond to DataInKeySpacePath not having Resolved on main (after rebase)
ScottDugas Oct 28, 2025
c255044
Cleanup some of the export tests
ScottDugas Oct 28, 2025
f30cb54
Fix minor typo
ScottDugas Oct 31, 2025
287ff3f
Reduce some duplication in import tests
ScottDugas Oct 31, 2025
2cac9f3
Remove unused parameter
ScottDugas Nov 9, 2025
77d85a2
Create better helpers to simplify test
ScottDugas Nov 9, 2025
afc98fb
Minor cleanup after self-review
ScottDugas Nov 10, 2025
7dfcb34
Instrument the time to import each data entry
ScottDugas Nov 13, 2025
e6b605e
Change KeySpacePathImportDataTest to have separate DB fields
ScottDugas Nov 13, 2025
7a4e127
Change tests that copyData to all be parameterized
ScottDugas Nov 13, 2025
40055dd
Test Importing a lot of data at once
ScottDugas Nov 13, 2025
4c9b11e
Add a test of importing a bunch of data
ScottDugas Nov 13, 2025
1a23763
Add comment to clearPath about how it doesn't clear DirectoryLayer
ScottDugas Nov 14, 2025
0fb43b9
Rename FDBDatabaseExtension.getDatabases -> getRandomDatabaseSubset
ScottDugas Nov 14, 2025
0ec5181
Test less large imports with the directory layer
ScottDugas Nov 14, 2025
30b9a01
Handle creating directory layer entries better
ScottDugas Nov 14, 2025
1f78a84
Add back timing IMPORT_DATA
ScottDugas Nov 17, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ public enum Events implements StoreTimer.Event {
RANGE_SET_CONTAINS("range set contains key"),
/** The amount of time checking if a {@link com.google.common.collect.RangeSet} is empty. */
RANGE_SET_IS_EMPTY("range set is empty"),
/** The amount of time importing a single KeyValue into a path. */
IMPORT_DATA("import KeyValue"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this being used?


/** The amount of time spent clearing the space taken by an index that has been removed from the meta-data. */
REMOVE_FORMER_INDEX("remove former index"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@
import com.apple.foundationdb.annotation.API;
import com.apple.foundationdb.record.RecordCoreArgumentException;
import com.apple.foundationdb.record.logging.LogMessageKeys;
import com.apple.foundationdb.tuple.ByteArrayUtil2;
import com.apple.foundationdb.tuple.Tuple;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.Arrays;
import java.util.Objects;

/**
* Class representing a {@link KeyValue} pair within in {@link KeySpacePath}.
Expand Down Expand Up @@ -67,4 +70,22 @@ public Tuple getRemainder() {
return remainder;
}

@Override
public boolean equals(final Object o) {
if (o == null || getClass() != o.getClass()) {
return false;
}
final DataInKeySpacePath that = (DataInKeySpacePath)o;
return Objects.equals(path, that.path) && Objects.equals(remainder, that.remainder) && Arrays.equals(value, that.value);
}

@Override
public int hashCode() {
return Objects.hash(path, remainder, Arrays.hashCode(value));
}

@Override
public String toString() {
return path + "+" + remainder + "->" + ByteArrayUtil2.loggable(value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -410,4 +410,26 @@ default RecordCursor<DataInKeySpacePath> exportAllData(@Nonnull FDBRecordContext
@Nonnull ScanProperties scanProperties) {
throw new UnsupportedOperationException("exportAllData is not supported");
}

/**
* Imports the provided data exported via {@link #exportAllData} into this {@code KeySpacePath}.
* <p>
* This will validate that any data provided in {@code dataToImport} has a path that should be in this path,
* or one of the sub-directories, if not, the future will complete exceptionally with
* {@link RecordCoreIllegalImportDataException}.
* If there is any data already existing under this path, the new data will overwrite if the keys are the same.
* This will use the logical values in the {@link DataInKeySpacePath#getPath()} and
* {@link DataInKeySpacePath#getRemainder()} to determine the key, rather
* than the raw key, meaning that this will work even if the data was exported from a different cluster.
* Note, this will not correct for any cluster-specific data, other than {@link DirectoryLayerDirectory} data;
* for example, if you have versionstamps, that data will not align on the destination.
* </p>
* @param context the transaction context in which to save the data
* @param dataToImport the data to be saved to the database
* @return a future to be completed once all data has been important.
*/
@API(API.Status.EXPERIMENTAL)
@Nonnull
CompletableFuture<Void> importData(@Nonnull FDBRecordContext context,
@Nonnull Iterable<DataInKeySpacePath> dataToImport);
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.apple.foundationdb.record.cursors.LazyCursor;
import com.apple.foundationdb.record.logging.LogMessageKeys;
import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext;
import com.apple.foundationdb.record.provider.foundationdb.FDBStoreTimer;
import com.apple.foundationdb.record.provider.foundationdb.KeyValueCursor;
import com.apple.foundationdb.subspace.Subspace;
import com.apple.foundationdb.tuple.ByteArrayUtil;
Expand Down Expand Up @@ -316,6 +317,51 @@ public RecordCursor<DataInKeySpacePath> exportAllData(@Nonnull FDBRecordContext
1);
}

@Nonnull
@Override
public CompletableFuture<Void> importData(@Nonnull FDBRecordContext context,
@Nonnull Iterable<DataInKeySpacePath> dataToImport) {
return toTupleAsync(context).thenCompose(targetTuple -> {
// We use a mapPipelined here to help control the rate of insertions into the directory layer if those
// are happening.
// DirectoryLayer operations are done in a separate transaction for two reasons:
// 1. To reduce conflicts
// 2. So that it can immediately be cached without having to do it in a post-commit hook, which can make
// things complicated
// So if we just spun off a future for every data item, they would conflict like crazy, and retrying all
// of those conflicts would cause this future to take way longer than if we pipeline.
// This shouldn't make much of a difference in the general case because almost all the directory layer
// lookups should be from cache.
final RecordCursor<Void> insertionWork = RecordCursor.fromIterator(dataToImport.iterator())
.mapPipelined(dataItem ->
dataItem.getPath().toTupleAsync(context).thenAccept(itemPathTuple -> {
// Validate that this data belongs under this path
if (!TupleHelpers.isPrefix(targetTuple, itemPathTuple)) {
throw new RecordCoreIllegalImportDataException(
"Data item path does not belong under target path",
"target", targetTuple,
"item", itemPathTuple);
}

// Reconstruct the key using the path and remainder
Tuple keyTuple = itemPathTuple;
if (dataItem.getRemainder() != null) {
keyTuple = keyTuple.addAll(dataItem.getRemainder());
}

// Store the data
byte[] keyBytes = keyTuple.pack();
byte[] valueBytes = dataItem.getValue();
context.ensureActive().set(keyBytes, valueBytes);
}),
1);
// Use forEach to force consuming the entire cursor, which will cause the inserts to happen
final CompletableFuture<Void> allInsertions = insertionWork.forEach(vignore -> { })
.whenComplete((vignore, e) -> insertionWork.close());
return context.instrument(FDBStoreTimer.Events.IMPORT_DATA, allInsertions);
});
}

/**
* Returns this path properly wrapped in whatever implementation the directory the path is contained in dictates.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,11 @@ public RecordCursor<DataInKeySpacePath> exportAllData(@Nonnull FDBRecordContext
@Nonnull ScanProperties scanProperties) {
return inner.exportAllData(context, continuation, scanProperties);
}

@Nonnull
@Override
public CompletableFuture<Void> importData(@Nonnull FDBRecordContext context,
@Nonnull Iterable<DataInKeySpacePath> dataToImport) {
return inner.importData(context, dataToImport);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* RecordCoreIllegalImportDataException.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.record.provider.foundationdb.keyspace;

import com.apple.foundationdb.record.RecordCoreArgumentException;

import javax.annotation.Nonnull;

/**
* Thrown if the data being imported into {@link KeySpacePath#importData} does not belong in that path.
*/
public class RecordCoreIllegalImportDataException extends RecordCoreArgumentException {
private static final long serialVersionUID = 1L;

public RecordCoreIllegalImportDataException(@Nonnull final String msg, @Nonnull final Object... keyValue) {
super(msg, keyValue);
}
}

Check warning on line 36 in fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/RecordCoreIllegalImportDataException.java

View check run for this annotation

fdb.teamscale.io / Teamscale | Findings

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/RecordCoreIllegalImportDataException.java#L30-L36

`RecordCoreIllegalImportDataException` has inheritance depth of 5 which is deeper than maximum of 2 https://fdb.teamscale.io/findings/details/foundationdb-fdb-record-layer?t=FORK_MR%2F3578%2FScottDugas%2Fkeyspace-import%3AHEAD&id=3C3B0BC4FA83FF44B4E2CE5FBF7DBFF5
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@

import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* Tests for {@link DataInKeySpacePath}.
Expand Down Expand Up @@ -118,4 +120,105 @@
assertThrows(RecordCoreArgumentException.class,
() -> new DataInKeySpacePath(testPath, null, valueBytes));
}

@Test
void testEquals() {
KeySpace root = new KeySpace(
new KeySpaceDirectory("test", KeyType.STRING, UUID.randomUUID().toString()));

KeySpacePath testPath = root.path("test");
Tuple remainder1 = Tuple.from("key1", "key2");
byte[] value1 = Tuple.from("value1").pack();

DataInKeySpacePath data1 = new DataInKeySpacePath(testPath, remainder1, value1);
DataInKeySpacePath data2 = new DataInKeySpacePath(testPath, remainder1, value1);

// Reflexive: object equals itself
assertEquals(data1, data1);

// Symmetric: a.equals(b) implies b.equals(a)

Check warning on line 139 in fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/DataInKeySpacePathTest.java

View check run for this annotation

fdb.teamscale.io / Teamscale | Findings

fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/DataInKeySpacePathTest.java#L139

Commented Out Code https://fdb.teamscale.io/findings/details/foundationdb-fdb-record-layer?t=FORK_MR%2F3578%2FScottDugas%2Fkeyspace-import%3AHEAD&id=4C343B505CA10EE0454A583C9453957B
assertEquals(data1, data2);
assertEquals(data2, data1);

// Test with different remainder
Tuple remainder2 = Tuple.from("different", "key");
DataInKeySpacePath data3 = new DataInKeySpacePath(testPath, remainder2, value1);
assertNotEquals(data1, data3);

// Test with different value
byte[] value2 = Tuple.from("value2").pack();
DataInKeySpacePath data4 = new DataInKeySpacePath(testPath, remainder1, value2);
assertNotEquals(data1, data4);

// Test with different path
KeySpace root2 = new KeySpace(
new KeySpaceDirectory("test", KeyType.STRING, UUID.randomUUID().toString()));
KeySpacePath testPath2 = root2.path("test");
DataInKeySpacePath data5 = new DataInKeySpacePath(testPath2, remainder1, value1);
assertNotEquals(data1, data5);

// Test with null remainder
DataInKeySpacePath data6 = new DataInKeySpacePath(testPath, null, value1);
DataInKeySpacePath data7 = new DataInKeySpacePath(testPath, null, value1);
assertEquals(data6, data7);
assertNotEquals(data1, data6);

// Test with null object
assertNotEquals(data1, null);

// Test with different class
assertNotEquals(data1, "not a DataInKeySpacePath");
}

@Test
void testHashCode() {
KeySpace root = new KeySpace(
new KeySpaceDirectory("test", KeyType.STRING, UUID.randomUUID().toString()));

KeySpacePath testPath = root.path("test");
Tuple remainder = Tuple.from("key1", "key2");
byte[] value = Tuple.from("value1").pack();

DataInKeySpacePath data1 = new DataInKeySpacePath(testPath, remainder, value);
DataInKeySpacePath data2 = new DataInKeySpacePath(testPath, remainder, value);

// Equal objects must have equal hash codes
assertEquals(data1.hashCode(), data2.hashCode());

// Test with null remainder
DataInKeySpacePath data3 = new DataInKeySpacePath(testPath, null, value);
DataInKeySpacePath data4 = new DataInKeySpacePath(testPath, null, value);
assertEquals(data3.hashCode(), data4.hashCode());

// Different objects should generally have different hash codes (not required, but good practice)
Tuple remainder2 = Tuple.from("different", "key");
DataInKeySpacePath data5 = new DataInKeySpacePath(testPath, remainder2, value);
assertNotEquals(data1.hashCode(), data5.hashCode());
}

@Test
void testToString() {
final String rootUuid = UUID.randomUUID().toString();
KeySpace root = new KeySpace(
new KeySpaceDirectory("test", KeyType.STRING, rootUuid));

KeySpacePath testPath = root.path("test");
Tuple remainder = Tuple.from("key1", "key2");
byte[] value = Tuple.from("value1").pack();

DataInKeySpacePath data = new DataInKeySpacePath(testPath, remainder, value);

String result = data.toString();

// Verify the string contains expected components
assertTrue(result.contains(rootUuid));
assertTrue(result.contains("test"));
assertTrue(result.contains("key1"), "toString should contain remainder elements");
assertTrue(result.contains("key2"), "toString should contain remainder elements");

// Test with null remainder
DataInKeySpacePath dataWithNullRemainder = new DataInKeySpacePath(testPath, null, value);
String resultWithNull = dataWithNullRemainder.toString();
assertTrue(resultWithNull.contains("null"), "toString should contain 'null' for null remainder");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
Expand All @@ -69,7 +68,7 @@ class KeySpacePathDataExportTest {
final FDBDatabaseExtension dbExtension = new FDBDatabaseExtension();

@Test
void exportAllDataFromSimplePath() throws ExecutionException, InterruptedException {
void exportAllDataFromSimplePath() {
KeySpace root = new KeySpace(
new KeySpaceDirectory("root", KeyType.STRING, UUID.randomUUID().toString())
.addSubdirectory(new KeySpaceDirectory("level1", KeyType.LONG)));
Expand All @@ -83,13 +82,14 @@ void exportAllDataFromSimplePath() throws ExecutionException, InterruptedExcepti

// Add data at different levels
for (int i = 0; i < 5; i++) {
Tuple key = basePath.add("level1", (long) i).toTuple(context);
final KeySpacePath path = basePath.add("level1", (long)i);
Tuple key = path.toTuple(context);
tr.set(key.pack(), Tuple.from("value" + i).pack());

// Add some sub-data under each key
for (int j = 0; j < 3; j++) {
Tuple subKey = key.add("sub" + j);
tr.set(subKey.pack(), Tuple.from("subvalue" + i + "_" + j).pack());
tr.set(path.toSubspace(context).pack(Tuple.from("sub" + j)),
Tuple.from("subvalue" + i + "_" + j).pack());
}
}
context.commit();
Expand All @@ -103,16 +103,19 @@ void exportAllDataFromSimplePath() throws ExecutionException, InterruptedExcepti
// Should have 5 main entries + 15 sub-entries = 20 total
assertEquals(20, allData.size());

assertThat(allData)
.allSatisfy(data ->
assertThat(data.getPath().getDirectoryName()).isEqualTo("level1"));

// Verify the data is sorted by key
for (int i = 1; i < allData.size(); i++) {
assertTrue(getKey(allData.get(i - 1), context).compareTo(getKey(allData.get(i), context)) < 0);
}
assertThat(allData.stream().map(data -> getKey(data, context)).collect(Collectors.toList()))
.isSorted();
}
}

// `toTuple` does not include the remainder, I'm not sure if that is intentional, or an oversight.
private Tuple getKey(final DataInKeySpacePath dataInKeySpacePath, final FDBRecordContext context) throws ExecutionException, InterruptedException {
final ResolvedKeySpacePath resolvedKeySpacePath = dataInKeySpacePath.getPath().toResolvedPathAsync(context).get();
private Tuple getKey(final DataInKeySpacePath dataInKeySpacePath, final FDBRecordContext context) {
final ResolvedKeySpacePath resolvedKeySpacePath = dataInKeySpacePath.getPath().toResolvedPathAsync(context).join();
if (dataInKeySpacePath.getRemainder() != null) {
return resolvedKeySpacePath.toTuple().addAll(dataInKeySpacePath.getRemainder());
} else {
Expand Down Expand Up @@ -524,9 +527,7 @@ private static void exportWithContinuations(final KeySpacePath pathToExport,
final RecordCursor<DataInKeySpacePath> cursor = pathToExport.exportAllData(context, continuation.toBytes(),
scanProperties);
final AtomicReference<RecordCursorResult<Tuple>> tupleResult = new AtomicReference<>();
final List<Tuple> batch = cursor.map(dataInPath -> {
return Tuple.fromBytes(dataInPath.getValue());
}).asList(tupleResult).join();
final List<Tuple> batch = cursor.map(dataInPath -> Tuple.fromBytes(dataInPath.getValue())).asList(tupleResult).join();
actual.add(batch);
continuation = tupleResult.get().getContinuation();
}
Expand Down Expand Up @@ -578,7 +579,7 @@ void exportAllDataThroughKeySpacePathWrapper() {
}

@Test
void exportAllDataThroughKeySpacePathWrapperResolvedPaths() {
void exportAllDataThroughKeySpacePathWrapperRemainders() {
final FDBDatabase database = dbExtension.getDatabase();
final EnvironmentKeySpace keySpace = EnvironmentKeySpace.setupSampleData(database);

Expand Down
Loading
Loading