Skip to content
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

Add miscelaneous haddock doc improvements to the normal API #10

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

dcoutts
Copy link
Collaborator

@dcoutts dcoutts commented Sep 18, 2023

In particular, discuss snapshots and duplicates: durability and persistent data structures.

Copy link
Collaborator

@jorisdral jorisdral left a comment

Choose a reason for hiding this comment

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

Left some comments about additions to the documentation.

To model these additional requirements, the model should have access to some "global state". For example, creating new sessions in the same folder should probably result in an exception, but the only way to model that is if we globally know which sessions are currently opened in which folders. See https://github.com/input-output-hk/lsm-tree/pull/4/files#diff-4c5ac7d802bb3e12d031dd04443db91f1a0e4a2f8115c0e3ff469887aba9d750

-- parts. A table handle is the object/reference by which an in-use LSM table
-- will be operated upon. In our API it identifies a single mutable instance of
-- parts. A table handle is the object\/reference by which an in-use LSM table
-- will be operated upon. In this API it identifies a single mutable instance of
-- an LSM table. The multiple-handles feature allows for there to may be many
-- such instances in use at once.
type TableHandle :: (Type -> Type) -> Type -> Type -> Type -> Type
Copy link
Collaborator

@jorisdral jorisdral Sep 18, 2023

Choose a reason for hiding this comment

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

Addition:

  • An operation on a table handle should throw an exception if the table handle is closed.
  • Closing a table handle should be idempotent IMO

Regarding sessions:

  • Once a session is closed, any table handles related to that session should be closed as well.
  • Using a session once it is closed, for example in new or open, should result in an exception
  • Opening two sessions in the same folder should probably also result in an exception. We might otherwise get name clashes for files. A session should ensure that file names are unique within that session, but when two sessions are open in the same folder, then both sessions might create files of the same name

@@ -272,7 +294,7 @@ snapshot ::
-> m ()
snapshot = undefined

-- | Open a table through a snapshot, returning a new table handle.
-- | Open a table from a named snapshot, returning a new table handle.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Addition:

  • Opening a snapshot that doesn't exist should result in an exception.
  • If we try to open a snapshot but the types of keys+values+blobs are not as expected, then an exception should be thrown. For example, if a snapshot was created from a TableHandle m Int Int Int, but we open it as TableHandle m Bool Bool Bool, we get an exception

Copy link
Collaborator

Choose a reason for hiding this comment

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

f we try to open a snapshot but the types of keys+values+blobs are not as expected

How that can be enforced? E.g. Typeable is not stable value (if type is moved to the different module, it will change).

Copy link
Collaborator

@jorisdral jorisdral Sep 20, 2023

Choose a reason for hiding this comment

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

We could use some other type of tag that could be written to disk, like a custom class

The alternative would be that a user gets a encoder/decoder error as soon as they try to manipulate/query a table with the wrong type

@dcoutts dcoutts mentioned this pull request Sep 18, 2023
@phadej
Copy link
Collaborator

phadej commented Sep 19, 2023

To model these additional requirements, the model should have access to some "global state". For example, creating new sessions in the same folder

I don't think it's worth modelling that. All model sessions can live independently of each other.

I.e. the Session type is not shared by the model.

... EDIT except if you want to make lockstep which opens/closes sessions often/all-the-time; but that is unrealistic usage pattern.

@jorisdral
Copy link
Collaborator

To model these additional requirements, the model should have access to some "global state". For example, creating new sessions in the same folder

I don't think it's worth modelling that. All model sessions can live independently of each other.

I.e. the Session type is not shared by the model.

... EDIT except if you want to make lockstep which opens/closes sessions often/all-the-time; but that is unrealistic usage pattern.

I suppose it's also fine if we do the extensive modelling only for the lockstep tests, and focus on the table model here instead. The latter is where all the interesting stuff happens anyway, and session/snapshot/handle administration is fairly straightforward.

My plan would not be to test opening/closing sessions all the time, but IMO it still makes sense to test a few use cases related to sessions/snapshots/handles administration, for example:

  • Using a handle after it is closed.
  • Using a session after it is closed.
  • Opening two sessions on the same path.
  • Opening a snapshot twice.
  • Etc.

In particular, discuss snapshots, duplicates, resource management and
concurrency.
It mangles the haddock sections.
See haskell/stylish-haskell#466

And fix one whitespace issue stylish-haskell found.
@dcoutts dcoutts merged commit 93a2cf6 into main Sep 26, 2023
15 checks passed
@dcoutts dcoutts deleted the dcoutts/haddocks branch September 26, 2023 14:24
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.

3 participants