Introduce KeySpacePath.importData to import previously exported data #3578
Conversation
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
ScottDugas
force-pushed
the
keyspace-import
branch
from
6a071ed to
abd67ac
Compare
ScottDugas
force-pushed
the
keyspace-import
branch
from
abd67ac to
287ff3f
Compare
ScottDugas
changed the title
ohadzeliger
requested changes
Nov 12, 2025
...main/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImpl.java
Show resolved
Hide resolved
| } | ||
|
|
||
| // Store the data | ||
| byte[] keyBytes = keyTuple.pack(); |
Contributor
There was a problem hiding this comment.
Should we add some fdb timer metrics for future use (imported_count)?
Collaborator
Author
There was a problem hiding this comment.
Yeah, I think a timer around importFuture makes sense.
...record-layer-core/src/test/java/com/apple/foundationdb/record/test/FDBDatabaseExtension.java
Outdated
Show resolved
Hide resolved
...com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImportDataTest.java
Outdated
Show resolved
Hide resolved
...com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImportDataTest.java
Outdated
Show resolved
Hide resolved
...main/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImpl.java
Outdated
Show resolved
Hide resolved
...com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImportDataTest.java
Outdated
Show resolved
Hide resolved
...com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImportDataTest.java
Show resolved
Hide resolved
...com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImportDataTest.java
Outdated
Show resolved
Hide resolved
|
|
||
| verifySingleKey(dataPath, Tuple.from("item"), Tuple.from("final_value")); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Additional potential tests:
- Large data (or any out of band error) during import
- Import into partial path (no leaves in import data) + some remainders
- import where data is of the wrong type
Collaborator
Author
There was a problem hiding this comment.
- Yes, a test of more data than can be inserted into a single transaction would make sense, but not if I move it to
ResolvedKeySpacePathand just have it take a singleDataInKeySpacePath. - I'm not sure what you mean by a partial path.
- If by
datayou mean the value, there is no validation, and it is notKeySpacePaths responsibility to know what is in the data. If you mean the object in the path, that should be validated above this call, and should be trust-worthy by the time you get aDataInKeySpacePath. Ideally this would be validated when you create theKeySpacePath, but it is covered in the serialization work, and I explain a bit more on the situation there: https://github.com/FoundationDB/fdb-record-layer/pull/3747/files#diff-15120b2e222e6bb7c2647b670f676b719cce8602e410487604bc87e9ea30a3b0R179
Contributor
There was a problem hiding this comment.
By the second bullet I meant importing into the middle of the path, as when you have a path defined for /company/employee/id/profile and the import only has /company/employee
In doing this, I had to rework the test for overwriting data, and in doing so, I decided it would be better to have 3 tests.
Now all will both run by copying back to the same cluster, and copying between clusters.
Also needed equals & hashCode & toString on DataInKeySpacePathTest so added tests for those
ohadzeliger
requested changes
Nov 17, 2025
| /** 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"), |
ohadzeliger
approved these changes
Nov 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
This introduces a new
KeySpacePath.importDatathat will importDataInKeySpacePathas gathered byKeySpacePath.exportAllData.The new method works when importing data exported from other clusters.
Resolves: #3573
Resolves: #3751 -- I thought I was going to pull this out, but went back and resolved it with a mapPipelined cursor.