Expiry works only during runtime and is lost on shrink

[boguslaw-wojcik]

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.

[Sora233]

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

[x528491x]

@tidwall please look into this issue.

[tidwall]

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]

I just pushed a fix that addresses the problem.

[boguslaw-wojcik]

Thank you @tidwall !

[boguslaw-wojcik]

I think I can close the issue as resolved.