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

Merge() seems to have a bug #258

Closed
taroim opened this issue Aug 17, 2023 · 13 comments
Closed

Merge() seems to have a bug #258

taroim opened this issue Aug 17, 2023 · 13 comments

Comments

@taroim
Copy link

taroim commented Aug 17, 2023

package main

import (
	"fmt"

	"github.com/rosedblabs/rosedb/v2"
	"github.com/rosedblabs/rosedb/v2/utils"
)

func main() {
	options := rosedb.DefaultOptions
	options.DirPath = "./tmp/rosedb_merge"

	// merge after insert
	db, err := rosedb.Open(options)
	if err != nil {
		panic(err)
	}

	// output:KeysNum=0, DiskSize=0.00MiB
	fmt.Printf("KeysNum=%v, DiskSize=%.2fMiB\n", db.Stat().KeysNum, float64(db.Stat().DiskSize)/1024/1024)

	for i := 0; i < 10; i++ {
		_ = db.Put([]byte(utils.GetTestKey(i)), utils.RandomValue(100*1024))
	}
	_ = db.Merge()
	_ = db.Close()

	// merge after deletion
	db, err = rosedb.Open(options)
	if err != nil {
		panic(err)
	}

	// output:KeysNum=10, DiskSize=0.98MiB
	fmt.Printf("KeysNum=%v, DiskSize=%.2fMiB\n", db.Stat().KeysNum, float64(db.Stat().DiskSize)/1024/1024)

	for i := 0; i < 10; i++ {
		_ = db.Delete([]byte(utils.GetTestKey(i)))
	}
	_ = db.Merge()
	_ = db.Close()

	// open statistics again
	db, err = rosedb.Open(options)
	if err != nil {
		panic(err)
	}

	// output:KeysNum=10, DiskSize=0.00MiB
	fmt.Printf("KeysNum=%v, DiskSize=%.2fMiB\n", db.Stat().KeysNum, float64(db.Stat().DiskSize)/1024/1024)

	_ = db.Close()
}

执行结果:
KeysNum=0, DiskSize=0.00MiB
KeysNum=10, DiskSize=0.98MiB
KeysNum=10, DiskSize=0.00MiB # 请注意:此处 KeysNum 应该为 0

问题描述:
Put 数据后执行 Merge(),然后 Delete 数据后再执行 Merge(),这时候就会出现所有的 Key 都还在!?

@Jeremy-Run
Copy link
Contributor

Jeremy-Run commented Aug 17, 2023

@taroim Thank you for your feedback, this PR can solve the problem you mentioned.

@facework
Copy link

In version 2.2.3, the Iterator method has been removed. Why?

@taroim
Copy link
Author

taroim commented Aug 17, 2023

@Jeremy-Run After updating to version 2.2.3, the execution result remains unchanged, and the issue doesn't seem to be resolved.

@taroim
Copy link
Author

taroim commented Aug 17, 2023

Sorry, I pulled the wrong branch, my bad.

@Jeremy-Run
Copy link
Contributor

Jeremy-Run commented Aug 17, 2023

In version 2.2.3, the Iterator method has been removed. Why?

@facework
The version 2.2.3 does not exist, do you mean 2.3.x? 2.3.x has not been released yet.
As to why the iterator is removed, this Issue should help you.

@Jeremy-Run
Copy link
Contributor

Jeremy-Run commented Aug 17, 2023

@Jeremy-Run After updating to version 2.2.3, the execution result remains unchanged, and the issue doesn't seem to be resolved.

@taroim pr hasn't been merged into the main branch yet, wait for @roseduan review.

@roseduan
Copy link
Collaborator

In version 2.2.3, the Iterator method has been removed. Why?

The iterator replies on the IRadix data structure, but after our tests, the IRadix will occupy lots of memory when the database grows larger.

So we decide to use BTree as the memory data structure instead of IRadix.
Then we add some similar methods to do the same thing with the previous iterator: Ascend and Descend.

So next release the iterator will be removed, if you have any troubles, please feel free to give us feedback.

@roseduan
Copy link
Collaborator

This problem has been resolved, please update to the latest commit, thanks. @taroim

@taroim
Copy link
Author

taroim commented Aug 18, 2023

It's usable, thank you.

@taroim taroim closed this as completed Aug 18, 2023
@taroim
Copy link
Author

taroim commented Aug 18, 2023

package main

import (
	"fmt"

	"github.com/rosedblabs/rosedb/v2"
	"github.com/rosedblabs/rosedb/v2/utils"
)

func main() {
	options := rosedb.DefaultOptions
	options.DirPath = "./tmp/rosedb_merge"

	// merge after insert
	db, err := rosedb.Open(options)
	if err != nil {
		panic(err)
	}

	for i := 0; i < 10; i++ {
		_ = db.Put([]byte(utils.GetTestKey(i)), utils.RandomValue(100*1024))
	}
	_ = db.Merge(true)
	_ = db.Close()

	// merge after deletion
	db, err = rosedb.Open(options)
	if err != nil {
		panic(err)
	}

	fmt.Printf("KeysNum=%v, DiskSize=%.2fMiB\n", db.Stat().KeysNum, float64(db.Stat().DiskSize)/1024/1024)
	for i := 0; i < 10; i++ {
		_ = db.Delete([]byte(utils.GetTestKey(i)))
	}
	_ = db.Merge(true)
	_ = db.Close()

	// open statistics again
	db, err = rosedb.Open(options)
	if err != nil {
		panic(err)
	}

	fmt.Printf("KeysNum=%v, DiskSize=%.2fMiB\n", db.Stat().KeysNum, float64(db.Stat().DiskSize)/1024/1024)
	_ = db.Close()
}

First Execution Result:
KeysNum=10, DiskSize=0.98MiB
KeysNum=0, DiskSize=0.00MiB

Second Execution Result:
KeysNum=10, DiskSize=0.00MiB // Here, DiskSize = 0 !???
KeysNum=0, DiskSize=0.00MiB

@taroim taroim reopened this Aug 18, 2023
@roseduan
Copy link
Collaborator

I will fix it later.
The Merge operation has some changes recently, so it may be unstable.

@roseduan
Copy link
Collaborator

package main

import (
	"fmt"

	"github.com/rosedblabs/rosedb/v2"
	"github.com/rosedblabs/rosedb/v2/utils"
)

func main() {
	options := rosedb.DefaultOptions
	options.DirPath = "./tmp/rosedb_merge"

	// merge after insert
	db, err := rosedb.Open(options)
	if err != nil {
		panic(err)
	}

	for i := 0; i < 10; i++ {
		_ = db.Put([]byte(utils.GetTestKey(i)), utils.RandomValue(100*1024))
	}
	_ = db.Merge(true)
	_ = db.Close()

	// merge after deletion
	db, err = rosedb.Open(options)
	if err != nil {
		panic(err)
	}

	fmt.Printf("KeysNum=%v, DiskSize=%.2fMiB\n", db.Stat().KeysNum, float64(db.Stat().DiskSize)/1024/1024)
	for i := 0; i < 10; i++ {
		_ = db.Delete([]byte(utils.GetTestKey(i)))
	}
	_ = db.Merge(true)
	_ = db.Close()

	// open statistics again
	db, err = rosedb.Open(options)
	if err != nil {
		panic(err)
	}

	fmt.Printf("KeysNum=%v, DiskSize=%.2fMiB\n", db.Stat().KeysNum, float64(db.Stat().DiskSize)/1024/1024)
	_ = db.Close()
}

First Execution Result: KeysNum=10, DiskSize=0.98MiB KeysNum=0, DiskSize=0.00MiB

Second Execution Result: KeysNum=10, DiskSize=0.00MiB // Here, DiskSize = 0 !??? KeysNum=0, DiskSize=0.00MiB

I have fixed this in the latest commit, enjoy!

Feel free to give us any feedback, thanks.

@taroim
Copy link
Author

taroim commented Aug 18, 2023

OK, thanks.

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

No branches or pull requests

4 participants