Store Benchmark Results in Dedicated CI Branch #1172
Conversation
WalkthroughThe CI workflow in Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Main as Main Repository
participant BM as Benchmark Repository
CI->>Main: Checkout current repository (path: repo)
CI->>BM: Checkout benchmark branch (branch: yorkie-ci-benchmark, path: benchmark-repo)
CI->>CI: Change directory to repo
CI->>CI: Execute benchmark command
CI->>BM: Read benchmark result files from benchmark-repo
CI->>BM: Create/update repository, add, commit, and push benchmark results
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (6)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
131-139: Robust Previous Benchmark Retrieval & Minor Formatting
The steps to read the previous benchmark result (by checking forbenchmark-repo/bench_result.txtandcommit_hash.txt) are implemented correctly. A small note: line 134 appears to have trailing spaces as flagged by static analysis. Please remove these trailing spaces to meet linting standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 134-134: trailing spaces
(trailing-spaces)
173-197: Storing Benchmark Results with Git Integration
The "Store benchmark result" step is well structured: it creates or reuses thebenchmark-repodirectory, copies over the benchmark output and commit hash, and commits these changes to the appropriate branch.A couple of suggestions:
- Local vs Global Git Config: Instead of using
git config --global, consider configuring Git locally (i.e., without the--globalflag) to avoid side effects in concurrent jobs.- Error Handling: While the script assumes smooth execution, adding error checks after critical Git commands (e.g., after
git commitandgit push) could improve robustness.For example, you might consider a diff like below to adjust the Git configuration locally:
- git config --global user.name "GitHub Actions" - git config --global user.email "[email protected]" + git config user.name "GitHub Actions" + git config user.email "[email protected]"Overall, the approach to store benchmark results in a dedicated branch is solid.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/ci.yml
[error] 134-134: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (4)
.github/workflows/ci.yml (4)
112-115: Clarify Checkout Path Usage
The "Checkout current repository" step sets a custom checkout path (repo), which is clear and helps isolate the workspace. Please ensure that all subsequent steps correctly reference files relative to this new path.
117-123: Benchmark Branch Checkout & Error Handling
The new "Checkout benchmark branch" step correctly checks out theyorkie-ci-benchmarkbranch into thebenchmark-repodirectory. The use ofcontinue-on-error: trueindicates that failures here are tolerated (e.g., if the branch does not yet exist). Verify that the later steps gracefully handle a missing or incomplete checkout.
141-144: Updated Stack Command with Custom Repository Path
The "Stack" step now runs the Docker Compose command using therepodirectory (i.e.,repo/build/docker/docker-compose.yml). This change is consistent with the new checkout path, but please double-check that any dependencies or file references in the Docker Compose file are updated accordingly.
144-151: Bench Step Execution in Isolated Directory
The "Bench" step changes into therepodirectory, executes the benchmark viamake bench, and processes the output file correctly. Ensure that the command generatesoutput.txtin the expected location and that usingjqto capture the result works reliably in all environments.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1172 +/- ##
=======================================
Coverage 38.28% 38.28%
=======================================
Files 171 171
Lines 25775 25775
=======================================
Hits 9867 9867
Misses 15075 15075
Partials 833 833 ☔ View full report in Codecov by Sentry. |
hackerwins
changed the title
What this PR does / why we need it:
This PR changes how we store and retrieve benchmark results in our CI pipeline. Instead of using GitHub Actions cache, we now store benchmark results in a
yorkie-ci-benchmarkbranch.Which issue(s) this PR fixes:
Fixes #1171
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit