Conversation
| - 100000 | ||
| - -uri | ||
| - {{.MONGODB_URI}} | ||
| - --runAll |
There was a problem hiding this comment.
Simple example from readme https://github.com/idealo/mongodb-benchmarking?tab=readme-ov-file#run-all-tests.
It takes about 1 min to run, do we want to tweak the parameters?
There was a problem hiding this comment.
Alternatively we can run insert, update, delete ops in separate config, that would allow ferretdb-postgresql to benchmark operations that do not return the error aggregate stage "$sample" is not implemented yet
There was a problem hiding this comment.
Pull Request Overview
Adds support for running and parsing results from the mongodb-benchmarking tool via a new mongobench runner, updates dependency versions (including the new submodule), and adjusts CI workflows to fetch submodules.
- Introduces a
mongoBenchrunner underinternal/runner/mongobenchwith parsing and tests. - Defines a generic
Measurementsinterface and updates config types to support multiple runner outputs. - Adds
mongodb-benchmarkingas a Git submodule, updatestools/go.mod, and enables submodule checkout in CI.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/go.mod | Added benchmarking tool, version bumps, local replace |
| .gitmodules | Updated submodule URL for mongodb-benchmarking |
| .github/workflows/go.yml & dance.yml | Enabled submodules: true for CI |
| internal/runner/mongobench/*.go/.yml/.test.go | New mongobench runner implementation & tests |
| internal/config/runner.go, configload, results.go | Added RunnerTypeMongoBench, Measurements interface |
| internal/runner/ycsb/ycsb.go | Refactored YCSB parser to use measurement type |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Pull Request is not mergeable
There was a problem hiding this comment.
Pull Request Overview
Adds a new mongobench runner to execute MongoDB benchmarking via the mongodb-benchmarking binary and integrate its results into the existing CI pipeline.
- Introduce a
mongobenchrunner implementation (parseFileNames,readResult,run,Run) - Extend configuration loading and CLI to support
RunnerTypeMongoBench - Add CI job
mongobench-runallunder.github/workflows/dance.yml
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| projects/mongobench-runall.yml | Defines a new benchmark suite with threads, docs, URI, and results mapping |
| internal/runner/mongobench/mongobench.go | Implements the runner logic, including parsing and reading results |
| internal/runner/mongobench/mongobench_test.go | Unit tests for parsing filenames and reading CSV results |
| internal/configload/runner.go | Adds YAML-to-config conversion for mongobench parameters |
| internal/configload/configload.go | Registers RunnerTypeMongoBench in the configuration loader |
| internal/config/runner.go | Defines RunnerTypeMongoBench constant and params struct |
| cmd/dance/main.go | Wires up mongobench.New in the CLI command switch |
| .github/workflows/dance.yml | Registers the new mongobench-runall job in CI |
Comments suppressed due to low confidence (1)
internal/runner/mongobench/mongobench.go:152
- The 'run' function orchestrating process execution, parsing filenames, and reading results currently lacks direct unit or integration tests. Consider adding tests that mock or stub exec.Command to verify end-to-end behavior and error paths.
func run(ctx context.Context, args []string, dir string) (map[string]config.TestResult, error) {
| for _, fileName := range fileNames { | ||
| var m map[string]float64 | ||
|
|
||
| if m, err = readResult(filepath.Join("..", "projects", fileName)); err != nil { |
There was a problem hiding this comment.
Using a hardcoded path '../projects' to locate result files may not align with the runner's working directory. Consider constructing the file path based on the configured 'dir' parameter or the actual command working directory.
| if m, err = readResult(filepath.Join("..", "projects", fileName)); err != nil { | |
| if m, err = readResult(filepath.Join(dir, "projects", fileName)); err != nil { |
Closes #1201.