buntdb icon indicating copy to clipboard operation
buntdb copied to clipboard

Expiry works only during runtime and is lost on shrink

Open boguslaw-wojcik opened this issue 2 years ago • 1 comments

Version: v1.3.0

If I add key first without expiry and then re-set it with expiry, but the expiry is not concluded during runtime, they key is never expired and on shrinking expiry is lost. To reproduce it:

Run this code:

func main()  {
	db, err := buntdb.Open("data.db")
	if err != nil {
		log.Fatal(err)
	}
	defer db.Close()

	err = db.Update(func(tx *buntdb.Tx) error {
		_, _, err := tx.Set("my_key", "my_val", nil)

		return err
	})

	err = db.Update(func(tx *buntdb.Tx) error {
		_, _, err := tx.Set("my_key", "my_val", &buntdb.SetOptions{Expires: true, TTL: 5 * time.Second})

		return err
	})
}

The data.db will look like this:

*3
$3
set
$6
my_key
$6
my_val
*5
$3
set
$6
my_key
$6
my_val
$2
ae
$10
1701350403

After more than 5 seconds, run this code:

func main() {
	db, err := buntdb.Open("data.db")
	if err != nil {
		log.Fatal(err)
	}
	defer db.Close()

	db.Shrink()

	db.View(func(tx *buntdb.Tx) error {
		err := tx.Ascend("", func(key, value string) bool {
			fmt.Printf("key: %s, value: %s\n", key, value)
			return true
		})

		return err
	})
}

It will print:

key: my_key, value: my_val

The data.db file looks like this:

*3
$3
set
$6
my_key
$6
my_val

I guess the expiry is not set when reconstructing the state from the log if there were previous versions of the same key without expiry.

boguslaw-wojcik avatar Nov 30 '23 13:11 boguslaw-wojcik

actually it's a bug about the db reconstruction, not only shrink

Sora233 avatar Jan 05 '24 17:01 Sora233

@tidwall please look into this issue.

x528491x avatar May 16 '24 01:05 x528491x

I looked into this issue. There are two problems.

First, when loading a database from disk, entries with an expired timestamp may still find their way to the in-memory database if there was a previous (non-expired) entry that was already added.

Second, iterators do not check each item that may have been expired before returning it to the caller. For example, calling Ascend will return potentially expired items, but calling Get will not.

The PR by @Sora233 sorta fixes the problem by always reinserting every item. This works for Get but not iterators like Ascend and fails to work for the example provided by @boguslaw-wojcik.

The correct fix would be to force delete entries on load when those entries are expired, and then also check when items have expired during iteration.

tidwall avatar May 17 '24 03:05 tidwall

I just pushed a fix that addresses the problem.

tidwall avatar May 17 '24 03:05 tidwall

Thank you @tidwall !

boguslaw-wojcik avatar May 17 '24 06:05 boguslaw-wojcik

I think I can close the issue as resolved.

boguslaw-wojcik avatar May 17 '24 06:05 boguslaw-wojcik