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 btree Ascend、Descend method and unitest. #257

Merged
merged 4 commits into from
Aug 15, 2023
Merged

add btree Ascend、Descend method and unitest. #257

merged 4 commits into from
Aug 15, 2023

Conversation

weijiew
Copy link
Contributor

@weijiew weijiew commented Aug 12, 2023

No description provided.

@weijiew weijiew mentioned this pull request Aug 12, 2023
index/btree.go Outdated
@@ -64,3 +64,23 @@ func (mt *MemoryBTree) Delete(key []byte) (*wal.ChunkPosition, bool) {
func (mt *MemoryBTree) Size() int {
return mt.tree.Len()
}

// Ascend calls f for each item in the tree in ascending order. If f returns false, iteration stops.
Copy link
Collaborator

@roseduan roseduan Aug 14, 2023

Choose a reason for hiding this comment

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

f -> handleFunc/handleFn maybe more clear?

The Caller can not use this function directly. We should add a function in db.go to get the value according to the position.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on your suggestion, I have made further modifications to the code. Could you please review it to see if it meets your expectations? If not, please point out any issues, and I will make further adjustments. Due to my daytime commitments for company projects, I can only work on the open-source project during the evenings. I apologize for any delay in responding.

Copy link
Collaborator

Choose a reason for hiding this comment

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

it`s so kind of you to tell me this, I also maintain the project in my spare time, so just feel free to contribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much for the project you've written, which has enabled me to further study the Bitcask model.

db.go Outdated
@@ -265,6 +265,30 @@ func (db *DB) Watch() (chan *Event, error) {
return db.watchCh, nil
}

// Ascend calls handleFn for each item in the tree in ascending order.
Copy link
Collaborator

@roseduan roseduan Aug 14, 2023

Choose a reason for hiding this comment

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

each item in the tree -> each key/value pair in the db

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

db.go Outdated
@@ -265,6 +265,30 @@ func (db *DB) Watch() (chan *Event, error) {
return db.watchCh, nil
}

// Ascend calls handleFn for each item in the tree in ascending order.
func (db *DB) Ascend(handleFn func(k []byte, v []byte) bool) error {
db.index.Ascend(func(key []byte, pos *wal.ChunkPosition) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. we can let the caller handle the error, so the handleFn should be func(k []byte, v []byte) (bool, error), and the Ascend has no return value.

  2. We must hold the read lock during the iteration. db.mu.RLock

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your guidance. I have made additional modifications. Could you please review it once more and let me know if there are any further changes needed?

@roseduan
Copy link
Collaborator

Thanks

@roseduan roseduan merged commit 453d928 into rosedblabs:main Aug 15, 2023
2 checks passed
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.

None yet

2 participants