Skip to content

Fix importSnapshot/exportSnapshot - #870

Open
wenkokke wants to merge 23 commits into
mainfrom
wip/wenkokke/import-export-2
Open

Fix importSnapshot/exportSnapshot#870
wenkokke wants to merge 23 commits into
mainfrom
wip/wenkokke/import-export-2

Conversation

@wenkokke

@wenkokke wenkokke commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

This PR addresses #853. It makes the following changes:

  • Revert the deprecation of withOpenSessionIO.
  • BREAKING: Replace the FsPath field of SnapshotImportDirDoesNotExistError/SnapshotExportDirExistsError with an FsErrorPath.
  • BREAKING: Rename the constructors of SnapshotImportDirDoesNotExistError/SnapshotExportDirExistsError to ErrSnapshotImportDirDoesNotExist/ErrSnapshotExportDirExists for consistency with the rest of the exceptions defined by the lsm-tree package.
  • BREAKING: Add an optional HasFS instance to the FsPath argument in importSnapshot/exportSnapshot. If the HasFS instance is provided, the FsPath path is interpreted as a path in the corresponding filesystem, and the snapshot is always copied. If the HasFS instance is not provided, the FsPath path is interpreted as a path in the filesystem for the database session, and the snapshot is hard linked with a fallback to copying.
  • Add the importSnapshotIO/exportSnapshotIO functions to Database.LSMTree, which accept a FilePath argument and always copy the snapshot files.
  • Add the importSnapshot/exportSnapshot functions to Database.LSMTree.Simple, which in turn call the importSnapshotIO/exportSnapshotIO functions and correctly repackage any exceptions.
  • Fix a bug where the SnapshotExportDirExistsError would contain the internal snapshot path.

@wenkokke
wenkokke force-pushed the wip/wenkokke/import-export-2 branch from 147d411 to be86e7f Compare July 27, 2026 15:53
@wenkokke
wenkokke force-pushed the wip/wenkokke/import-export-2 branch from be86e7f to e1b7c75 Compare July 27, 2026 15:53
@wenkokke
wenkokke force-pushed the wip/wenkokke/import-export-2 branch from e1b7c75 to ceb6d62 Compare July 27, 2026 16:10

@jorisdral jorisdral left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There are two general things to think about here:

  • If I create a session with withOpenSessionIO, then I can still use importSnapshot/exportSnapshot without providing a destination FS, which gets us back in unsafe territory where paths are interpreted with respect to the session directory. Maybe we can track a simple Bool in the Session type that tracks whether the session was created in withOpenSessionIO, and if it was, throw an error when a desination FS is not provided in importSnapshot/exportSnapshot

  • The hardlink with copy fallback is nice but has its downsides. it's not visible to the user if the fallback was used, or whether the files were hard linked. This is important to know, because changes to hard linked files are visible to all links, but not if the files were copied. Moreover, I think users will want to only hard link or only copy. So maybe we should have separate functions:

    • importSnapshotCopy (and importSnapshotCopyIO)
    • exportSnapshotCopy (and exportSnapshotCopyIO)
    • importSnapshotHardlink
    • exportSnapshotHardlink

    I intentionally left out IO versions of the hard linking import/export functions, because we want to only allow copies for sessions that are created with withOpenSessionIO. And maybe we should then add a new public error that captures the EXDEV error, so that users can write something like this on top of the lsm-tree library: try hard link, and if EXDEV, copy

Comment thread lsm-tree/src-core/Database/LSMTree/Internal/FS.hs Outdated
Comment thread lsm-tree/src-core/Database/LSMTree/Internal/FS.hs
Comment thread lsm-tree/src/Database/LSMTree/Simple.hs
Comment thread lsm-tree/src/Database/LSMTree/Simple.hs Outdated
Comment thread lsm-tree/src/Database/LSMTree/Simple.hs
Comment thread lsm-tree/src/Database/LSMTree.hs
Comment thread lsm-tree/src/Database/LSMTree.hs
Comment thread lsm-tree/src/Database/LSMTree.hs Outdated
Comment thread lsm-tree/src/Database/LSMTree.hs
Comment thread lsm-tree/CHANGELOG.md
@wenkokke

Copy link
Copy Markdown
Collaborator Author
  • If I create a session with withOpenSessionIO, then I can still use importSnapshot/exportSnapshot without providing a destination FS, which gets us back in unsafe territory where paths are interpreted with respect to the session directory. Maybe we can track a simple Bool in the Session type that tracks whether the session was created in withOpenSessionIO, and if it was, throw an error when a desination FS is not provided in importSnapshot/exportSnapshot

There's much more unsafe stuff in the library that is only guarded by a library. I think a well written warning is sufficient, which I've added.

@wenkokke

Copy link
Copy Markdown
Collaborator Author
  • The hardlink with copy fallback is nice but has its downsides. it's not visible to the user if the fallback was used, or whether the files were hard linked. This is important to know, because changes to hard linked files are visible to all links, but not if the files were copied.

ls -l tells you if a file has hard links, so the user can tell.

@wenkokke
wenkokke force-pushed the wip/wenkokke/import-export-2 branch from 015d2a2 to 79b5061 Compare July 30, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants