Added rnd read capabilities to bbolt bench

[ambaxter]

I was doing some testing and noticed that the benchmark functionality didn't have random access capabilities. This PR adds those options.

% ./bbolt bench -read-mode rnd                              
starting write benchmark.
Starting write iteration 0
Finished write iteration 0
starting read benchmark.
Completed 2195132 requests, 2195117/s 
# Write 1000(ops)       66.425456ms     (66.425µs/op)   (15054 op/sec)
# Read  2196000(ops)    1.000481397s    (455ns/op)      (2197802 op/sec)

./bbolt bench -read-mode rnd -write-mode rnd-nest         
starting write benchmark.
Starting write iteration 0
Finished write iteration 0
starting read benchmark.
Completed 1470719 requests, 1470489/s 
# Write 1000(ops)       67.886901ms     (67.886µs/op)   (14730 op/sec)
# Read  1471000(ops)    1.000507272s    (680ns/op)      (1470588 op/sec)

[ivanvc]

Hi, @ambaxter. Thanks for the pull request. Please ensure your commit is signed so the developer certificate of origin (DCO) check passes, i.e:

git rebase HEAD~1 --signoff
git push --force

@ahrtr, there will be conflicts with the migration to cobra-style of the bench command (#680). I removed read-mode as seq wasn't available when I rewrote the command. Do we want to wait until after 1.4 to add this PR? Then, it would be better to integrate it in #680. Otherwise, I can update my PR to add this functionality.

[ambaxter]

Hi, @ambaxter. Thanks for the pull request. Please ensure your commit is signed so the developer certificate of origin (DCO) check passes, i.e:

git rebase HEAD~1 --signoff
git push --force

Done

[ahrtr]

This looks like a useful enhancement. Since it doesn't break existing user experience, so it's accepted to add it in 1.4.0.

For rnd read, we should exclude the time of collecting & shuffle all the keys.

Obviously you did not sign the commit correctly. Please read https://github.com/etcd-io/bbolt/pull/711/checks?check_run_id=23409172937

[ivanvc]

This looks like a useful enhancement. Since it doesn't break existing user experience, so it's accepted to add it in 1.4.0.

Ok, I'll update my PR after this one gets merged.

[ambaxter]

Obviously you did not sign the commit correctly. Please read https://github.com/etcd-io/bbolt/pull/711/checks?check_run_id=23409172937

My apologies. I did reset my local git config and force pushed it with the commit ending in 'Signed-off-by: Adam Baxter [email protected]'. The DCO action is still failing with the old commit message, however.

[ivanvc]

@ambaxter You need to fix (and reset) your author configuration. It doesn't match the DCO.

Author: abaxter <Adam Baxter>
Date:   Tue Mar 26 13:00:01 2024 +0000

    Added rnd read capabilities to bbolt bench
    
    Signed-off-by: Adam Baxter <[email protected]>

Emphasis on Author.

[ambaxter]

@ambaxter You need to fix (and reset) your author configuration. It doesn't match the DCO.

Done. I'm sorry! This is probably the first big project I've committed to.

[ahrtr]

cc @fuweid @ivanvc @tjungblu PTAL, thx

[ahrtr]

Can you compare the benchmark result of rnd and seq reads?

Probably the collectKeys may have impact on the benchmark result of random read, because the OS may cache some data, accordingly the following random read actually just reads the data from cache instead of disk. One possible solution is to cache all the keys (and buckets for nested case) when writing instead of right before reading.

[ambaxter]

Can you compare the benchmark result of rnd and seq reads?

Probably the collectKeys may have impact on the benchmark result of random read, because the OS may cache some data, accordingly the following random read actually just reads the data from cache instead of disk. One possible solution is to cache all the keys (and buckets for nested case) when writing instead of right before reading.

I tested 100,000 and 1,000,000 with varying batch sizes. You're correct. Collecting the keys just before reading in general has a performance benefit.

[ambaxter]

The Robustness tests are currently failing with OOM.

[ambaxter]

Should be fixed now.

[ivanvc]

/ok-to-test

[ivanvc]

@ambaxter, can you please squash your commits? Thanks.

[ambaxter]

@ambaxter, can you please squash your commits? Thanks.

Done

[ivanvc]

@ahrtr PTAL

[ambaxter]

I'm not sure why the ARM64 test failed

[ahrtr]

lgtm

Thanks.

[ahrtr]

Please also consider adding a test to cover the new rnd read, to avoid error something like #711 (comment). You can do it in a followup PR or as the second commit in this PR.

[ambaxter]

Please also consider adding a test to cover the new rnd read, to avoid error something like #711 (comment). You can do it in a followup PR or as the second commit in this PR.

@ahrtr Sure. Like https://github.com/etcd-io/bbolt/blob/main/cmd/bbolt/main_test.go#L483 ?

[ahrtr]

@ahrtr Sure. Like https://github.com/etcd-io/bbolt/blob/main/cmd/bbolt/main_test.go#L483 ?

Yes, you can add a couple of more items into the list.

bbolt/cmd/bbolt/main_test.go

Lines 486 to 489 in 6716581

	}{
	"no-args": {},
	"100k count": {[]string{"-count", "100000"}},
	}

[ahrtr]

@fuweid do you have bandwidth to take a look at this PR, so that we can merge it? thx

[fuweid]

LGTM