search

etcd-io / bbolt

An embedded key/value database for Go.
https://go.etcd.io/bbolt
MIT License
7.88k stars 620 forks source link

Introduce benchmarking for PRs #739

Open ivanvc opened 2 months ago

ivanvc commented 2 months ago

Formalizing the comment from PR https://github.com/etcd-io/bbolt/pull/691#issuecomment-2073101869. Adding a GitHub action or Prow Job to compare benchmarks vs. main would help catch issues sooner, like the one tracked in #720.

@ahrtr, a couple of questions to finalize scoping this task:

ahrtr commented 1 month ago

Thanks @ivanvc for raising this issue. Please see my response below,

  • What would be a good set of benchmark parameters to run?

For the writing test, I think we should commit the TXN at least multiple times, i.e 10 times. So options.Iterations/options.BatchSize > 10. For the options.BatchSize, it should be big enough, i.e. at least 10K.

  • Should this run on GitHub Actions or as a Prow Job?

We can add it on github action firstlyh. Eventually we may want to migrate all workflow checks to Prow.

What would be the priority for this task?

It may not be a urgent task.

fuweid commented 1 month ago

Eventually we may want to migrate all workflow checks to Prow.

Is it any reason to move workflow to Prow?

ahrtr commented 1 month ago

It's part of the effort of https://github.com/kubernetes/k8s.io/issues/6102.

At least the Prow has more powerful machine, we can have shorter running time for the performance test. Please see https://github.com/etcd-io/bbolt/pull/691

fuweid commented 1 month ago

It's part of the effort of https://github.com/kubernetes/k8s.io/issues/6102.

At least the Prow has more powerful machine, we can have shorter running time for the performance test. Please see https://github.com/etcd-io/bbolt/pull/691

I was thinking that github action can provide VM which supports nested virtualization and we can simulate real power-failure. And it's more easy to debug CI changes in GitHub action. Just my two cents. Anyway, thanks for sharing the background.

ivanvc commented 1 week ago

The initial implementation in #750 is benchmarking only the default write and read modes (write=rnd, read=seq).

@ahrtr, should we expand the benchmarks to other combinations?

ahrtr commented 6 days ago

@ahrtr, should we expand the benchmarks to other combinations?

Yes, it's a good point. etcd is actually sequentially writing the keys. We need to consider different write modes and read modes, also different key & value sizes. Usually K8s's value size is bigger than 1K+? Could you draft a simple doc /doc/benchmark.md firstly? thx