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.

Sign up for GitHub

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

Jump to bottom

Add incremental functionality for the ordinary index #299

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Add incremental functionality for the ordinary index #299

merged 1 commit into from
Aug 12, 2024

Conversation

jeltsch
Copy link
Collaborator

@jeltsch jeltsch commented Jul 17, 2024 •
edited
Loading

This adds support for incremental construction and serialization of ordinary indexes.

Note that the branch of this pull request is currently stacked onto the branch of #296, as it depends on it. Once #296 is merged, I shall rebase the branch of the present pull request onto the then current main.

@jeltsch jeltsch added the enhancement New feature or request label Jul 17, 2024
@jeltsch jeltsch requested a review from mheinzel as a code owner July 17, 2024 17:30
@jeltsch jeltsch self-assigned this Jul 17, 2024
@jeltsch jeltsch force-pushed the jeltsch/ordinary-index-incremental branch from 50c287f to 454f5f2 Compare July 18, 2024 20:50
jorisdral
jorisdral reviewed Jul 21, 2024
View reviewed changes
src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated Show resolved Hide resolved
src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated Show resolved Hide resolved
src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated Show resolved Hide resolved
src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated Show resolved Hide resolved
src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated Show resolved Hide resolved
src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated Show resolved Hide resolved
src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated Show resolved Hide resolved
src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated Show resolved Hide resolved
src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated Show resolved Hide resolved
src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated Show resolved Hide resolved
@jorisdral
Copy link
Collaborator

Note that the branch of this pull request is currently stacked onto the branch of #296, as it depends on it. Once #296 is merged, I shall rebase the branch of the present pull request onto the then current main.

You can set #296 as the target branch for this PR, and then the PR only shows the changes with respect to #296. When #296 is merged, the target branch automatically changes to main and you won't have to rebase

@jeltsch jeltsch force-pushed the jeltsch/ordinary-index-incremental branch from 454f5f2 to b51efef Compare August 2, 2024 15:00
@jeltsch jeltsch changed the base branch from main to jeltsch/general-purpose-chunks August 2, 2024 18:43
@jeltsch
Copy link
Collaborator Author

jeltsch commented Aug 2, 2024

You can set #296 as the target branch for this PR, and then the PR only shows the changes with respect to #296.

Done. 🙂

@jeltsch jeltsch force-pushed the jeltsch/ordinary-index-incremental branch from 87a9d4d to b504107 Compare August 5, 2024 14:47
@jeltsch jeltsch force-pushed the jeltsch/general-purpose-chunks branch from b9c3c88 to a7b8c45 Compare August 5, 2024 16:09
@jeltsch jeltsch force-pushed the jeltsch/ordinary-index-incremental branch from b504107 to 6ff1394 Compare August 5, 2024 16:13
Base automatically changed from jeltsch/general-purpose-chunks to main August 5, 2024 18:41
@jeltsch jeltsch force-pushed the jeltsch/ordinary-index-incremental branch from 6ff1394 to 3cd44c1 Compare August 5, 2024 19:35
jorisdral
jorisdral approved these changes Aug 9, 2024
View reviewed changes
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.

LGTM. Let's merge after you've squashed the commits, and after you've resolved my last comment.

src/Database/LSMTree/Internal/IndexOrdinaryAcc.hs Outdated
Mutable.write buffer keyCount key
writeSTRef keyCountRef $! succ keyCount
feedBaler (keyListElem key) baler
AppendMultiPage key pageCount -> do
Copy link
Collaborator

Choose a reason for hiding this comment

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

pageCount is not the number of pages, it's the number of overflow pages. For AppendMultiPage _ noverflow, pagecount = noverflow + 1.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, you’re right. Fixed in a4fcb49.

@jeltsch jeltsch force-pushed the jeltsch/ordinary-index-incremental branch from a4fcb49 to 1b3158b Compare August 10, 2024 14:26
@jeltsch
Copy link
Collaborator Author

jeltsch commented Aug 10, 2024

LGTM. Let's merge after you've squashed the commits, and after you've resolved my last comment.

Squashed into 1b3158b. Ready to be merged.

@jorisdral jorisdral added this pull request to the merge queue Aug 12, 2024
Merged via the queue into main with commit 9d3d730 Aug 12, 2024
17 checks passed
@jorisdral jorisdral deleted the jeltsch/ordinary-index-incremental branch August 12, 2024 14:04
@jeltsch jeltsch mentioned this pull request Aug 12, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorisdral jorisdral jorisdral approved these changes

@dcoutts dcoutts Awaiting requested review from dcoutts dcoutts is a code owner

@mheinzel mheinzel Awaiting requested review from mheinzel mheinzel is a code owner

Assignees

@jeltsch jeltsch

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jeltsch @jorisdral