Skip to content

Extract a blockio package from lsm-tree #648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jorisdral
Copy link
Collaborator

@jorisdral jorisdral commented Mar 27, 2025

Both the HasBlockIO API and simulation will have to be exposed in some manner to users of the lsm-tree library. The former so that users can open a session, and the latter to make sure that users can also run lsm-tree with a file system simulation. Recall openSession:

openSession ::
forall m h. (IOLike m, Typeable h)
=> Tracer m Internal.LSMTreeTrace
-> HasFS m h
-> HasBlockIO m h -- TODO: could we prevent the user from having to pass this in?
-> FsPath -- ^ Path to the session directory
-> m (Session m)
openSession tr hfs hbio dir = Internal.Session' <$> Internal.openSession tr hfs hbio dir

^ the answer to the TODO here is: I don't think that in general we can prevent the user from having to pass in HasBlockIO. Initialising a HasFS is linked to initialising a HasBlockIO, but the way in which we do this for the real file system and simulated file system is different:

-- | Platform-dependent IO instantiation of 'HasBlockIO'.
ioHasBlockIO ::
HasFS IO HandleIO
-> IOCtxParams
-> IO (HasBlockIO IO HandleIO)
ioHasBlockIO = I.ioHasBlockIO

simHasBlockIO ::
(MonadCatch m, MonadMVar m, PrimMonad m, MonadSTM m)
=> StrictTMVar m MockFS
-> m (HasFS m HandleMock, HasBlockIO m HandleMock)
simHasBlockIO var = do
let hfs = simHasFS var
hbio <- fromHasFS hfs
pure (hfs, hbio)

So we can't hide HasBlockIO from the user, it seems. And, as @wenkokke suggested, we should probably expose the API and simulation in a separate package.

I had initially hoped we could re-export the necessary blockio definitions from the main lsm-tree library while keeping the sub-libraries hidden. However, since we shouldn't export the simulation (with dependencies on test libraries) from the public API, we would have to create a public sub-library with just the blockio simulation. At this point, it's probably better to have a separate package.

BWT, I also made blockio-sim a public sub-library of blockio-api (i.e., now the component is identified by blockio:sim). The reasoning here is that there are very few definitions in blockio-sim, and one would have to depend on blockio-api anyway to do anything with the simulation. As such, you could argue it's better not to separately publish blockio-sim. As far as I understand, Hackage has also gotten better at support for sub-libraries, see https://hackage.haskell.org/package/io-classes for example.

@jorisdral jorisdral force-pushed the jdral/extract-blockio-package branch from d9b57f6 to 03a580e Compare March 27, 2025 11:51
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I should put some text here if we want to proceed with this PR

@jorisdral jorisdral force-pushed the jdral/extract-blockio-package branch from 03a580e to fa5135c Compare March 27, 2025 11:55
@jorisdral jorisdral marked this pull request as ready for review March 27, 2025 12:36
@jorisdral jorisdral self-assigned this Mar 27, 2025
@jorisdral jorisdral force-pushed the jdral/extract-blockio-package branch 2 times, most recently from f69ff16 to 0af1136 Compare April 8, 2025 15:05
@jorisdral jorisdral force-pushed the jdral/extract-blockio-package branch 2 times, most recently from fb8f5a7 to bb332be Compare April 18, 2025 19:05
@jorisdral jorisdral force-pushed the jdral/extract-blockio-package branch from bb332be to 8c9cdc2 Compare April 23, 2025 15:06
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.

1 participant