Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fine-tune CI Workflows in PR #964

Merged
merged 10 commits into from
Aug 15, 2024
Merged

Fine-tune CI Workflows in PR #964

merged 10 commits into from
Aug 15, 2024

Conversation

binary-ho
Copy link
Contributor

@binary-ho binary-ho commented Aug 14, 2024 •
edited by sejongk
Loading

1. What this PR does / why we need it:

The execution time of the jobs included in the CI was too long. We had to wait longer to receive results due to unnecessary tests.

  • Build and Integration Test: 8 mins
  • Benchmark: 5 mins
  • Sharding Test: 3 mins

To prevent unnecessary CI tests from running, each CI test now only runs when there are changes related to the corresponding code.

  1. Now, you can check the CI results faster because only the relevant tests for the changed code are executed.
  2. You can easily verify whether a test was executed in the summary. (Refer to the image below)
    • Green check: Job executed
    • Red X: Job failed
    • Gray circle: Test did not run because it wasn’t the target package

image


Target packages:

  • build: all (except those ignored by paths-ignore)
  • benchmark: 'pkg/', 'server/packs/'
  • sharding test: 'server/backend/database/**'



2. Which issue(s) this PR fixes

Fine-tune CI Workflows in PR fixes #954



3. Special notes for your reviewer

Please check the target packages for each CI job.

Although I tried to classify them on my own, there are a few parts that feel ambiguous.


3.1 Build test target

There’s no specific target in the CI yml file, but I added yorkie/design/** to paths-ignore.

If there was a particular reason for not including the design package in the related issue #873, please let me know!

As a result, changes to the following targets will be ignored. (No CI tests will run)

If there are any other targets that should be added, please let me know.

  pull_request:
    branches: [ main ]
    paths-ignore:
      - 'api/docs/**'
      - 'build/charts/**'
      - 'design/**'
      - '**/*.md'
      - '**/*.txt'
      - '**/.gitignore'

Additionally, a job has been added to check the packages affected by changes.


jobs:
  ci-target-check:
    runs-on: ubuntu-latest

    outputs:
      build: ${{ steps.ci-target-check.outputs.build }}
      bench: ${{ steps.ci-target-check.outputs.bench }}
      sharding-test: ${{ steps.ci-target-check.outputs.sharding-test }}

    steps:
    - name: CI target check by path
      uses: dorny/paths-filter@v3
      id: ci-target-check
      with:
        filters: |
          build: '**'
          bench: 
            - 'pkg/**'
            - 'server/packs/**'
          sharding-test:
            - 'server/backend/database/**'



And each job checks the flag values as follows and executes the job if the value is true.

build:
    needs: ci-target-check
    if: ${{ needs.ci-target-check.outputs.build == 'true' }}

bench:
    needs: ci-target-check
    if: ${{ needs.ci-target-check.outputs.bench == 'true' }}

sharding_test:
    needs: ci-target-check
    if: ${{ needs.ci-target-check.outputs.sharding-test == 'true' }}



3.2 Bench test target

By default, all packages under yorkie/pkg are included. I marked these with O in the diagram below.

The parts that felt ambiguous are marked with ? in the diagram, and I’ve left additional questions.


image


3.2.1 (Question) Packages that need confirmation

Should the following packages be included or excluded in the benchmark targets?

  • yorkie/client package → Since it mainly serves as a layer for making calls like yorkie/api, I want to exclude it. (I have excluded it for now.)
  • yorkie/server/packs package → This package defines functions for pushing or pulling changes. I believe only server/packs within the server package should be included in the bench target. Is this correct? (I have included it for now.)

3.2.2 Packages excluded from the bench test target

  • yorkie/api → Excluded from the bench target since it only makes API calls
  • yorkie/cmd, yorkie/internal, yorkie/design → Excluded from the bench target
  • yorkie/server (excluding the yorkie/server/packs package)

3.3 Shard test target

  • yorkie/server/backend/database

3.4 Test Result

I tested to ensure everything works correctly.

3.4.1 Build test

Only the build will be executed when packages unrelated to CRDT and shard-test are modified.

→ Github Action Result: https://github.com/yorkie-team/yorkie/actions/runs/10387699150

image



3.4.2 Bench test

After modifying the packs.go file, both the build test and benchmark were executed out of the three jobs.

(The build failure was due to adding a TODO comment for testing purposes)

→ Github Action Result: https://github.com/yorkie-team/yorkie/actions/runs/10387811024

image



3.4.3 Sharding-test

After modifying the server/backend/database/memory/database.go file, the sharding test was additionally executed.

(The build failure was due to adding a TODO comment for testing purposes)

→ Github Action Result: https://github.com/yorkie-team/yorkie/actions/runs/10387851119

image

Summary by CodeRabbit

  • New Features

    • Introduced a new job in the CI workflow for target checks, optimizing the build process by conditionally executing jobs based on relevant file changes.

  • Improvements

    • Streamlined job steps for clarity and efficiency, ensuring that only necessary builds and tests run, enhancing overall CI performance.

@binary-ho binary-ho self-assigned this Aug 14, 2024
@coderabbitai coderabbitai
Copy link

coderabbitai bot commented Aug 14, 2024 •
edited
Loading

Walkthrough

The CI workflow has been significantly enhanced to improve efficiency and clarity. A new job, ci-target-check, has been introduced to conditionally determine which parts of the codebase require builds, benchmarks, or tests based on recent changes. This optimization results in streamlined execution of the build, bench, and sharding-test jobs, ensuring they only run when relevant files are modified. Overall, these modifications foster a more responsive and maintainable CI process.

Changes

Files Change Summary
.github/workflows/ci.yml Added ci-target-check job to optimize CI workflow; reorganized steps; improved conditional execution for existing jobs (build, bench, sharding-test); clarified linting steps.

Poem

🐇 In the land of code, we hop and play,
With new checks in place, we brighten the day.
Builds run swift, tests dance with delight,
A streamlined CI, oh what a sight!
Bouncing through paths, we’re agile and free,
Celebrating changes, just you wait and see! 🌟

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot]
coderabbitai bot reviewed Aug 14, 2024
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 80c6ea0 and df19964.

Files selected for processing (1)
  • .github/workflows/ci.yml (2 hunks)
Additional comments not posted (4)
.github/workflows/ci.yml (4)

20-40: Well-structured target check job.

The ci-target-check job effectively uses dorny/paths-filter to conditionally trigger subsequent jobs based on file changes. This is a good practice to optimize CI workflows.

40-85: Efficient build job with conditional execution.

The build job is well-organized with conditional execution based on the ci-target-check outputs. This structure enhances efficiency by preventing unnecessary builds.

90-139: Well-implemented benchmark job with caching.

The bench job's conditional execution and use of dorny/paths-filter for benchmark targets are effective. Caching previous benchmark data is a valuable addition for performance analysis.

140-183: Comprehensive sharding test job.

The sharding_test job is well-structured with clear steps for initializing sharding components and running tests. Conditional execution based on ci-target-check outputs enhances efficiency.

github-actions[bot]
github-actions bot reviewed Aug 14, 2024
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go Benchmark

Benchmark suite Current: 17ca7d9 Previous: ab50d54 Ratio
BenchmarkDocument/constructor_test 1508 ns/op 1337 B/op 24 allocs/op 1488 ns/op 1337 B/op 24 allocs/op 1.01
BenchmarkDocument/constructor_test - ns/op 1508 ns/op 1488 ns/op 1.01
BenchmarkDocument/constructor_test - B/op 1337 B/op 1337 B/op 1
BenchmarkDocument/constructor_test - allocs/op 24 allocs/op 24 allocs/op 1
BenchmarkDocument/status_test 966.4 ns/op 1305 B/op 22 allocs/op 949.6 ns/op 1305 B/op 22 allocs/op 1.02
BenchmarkDocument/status_test - ns/op 966.4 ns/op 949.6 ns/op 1.02
BenchmarkDocument/status_test - B/op 1305 B/op 1305 B/op 1
BenchmarkDocument/status_test - allocs/op 22 allocs/op 22 allocs/op 1
BenchmarkDocument/equals_test 8374 ns/op 7273 B/op 132 allocs/op 7592 ns/op 7273 B/op 132 allocs/op 1.10
BenchmarkDocument/equals_test - ns/op 8374 ns/op 7592 ns/op 1.10
BenchmarkDocument/equals_test - B/op 7273 B/op 7273 B/op 1
BenchmarkDocument/equals_test - allocs/op 132 allocs/op 132 allocs/op 1
BenchmarkDocument/nested_update_test 18438 ns/op 12139 B/op 262 allocs/op 17373 ns/op 12138 B/op 262 allocs/op 1.06
BenchmarkDocument/nested_update_test - ns/op 18438 ns/op 17373 ns/op 1.06
BenchmarkDocument/nested_update_test - B/op 12139 B/op 12138 B/op 1.00
BenchmarkDocument/nested_update_test - allocs/op 262 allocs/op 262 allocs/op 1
BenchmarkDocument/delete_test 23236 ns/op 15364 B/op 341 allocs/op 25256 ns/op 15363 B/op 341 allocs/op 0.92
BenchmarkDocument/delete_test - ns/op 23236 ns/op 25256 ns/op 0.92
BenchmarkDocument/delete_test - B/op 15364 B/op 15363 B/op 1.00
BenchmarkDocument/delete_test - allocs/op 341 allocs/op 341 allocs/op 1
BenchmarkDocument/object_test 8822 ns/op 6817 B/op 120 allocs/op 8560 ns/op 6817 B/op 120 allocs/op 1.03
BenchmarkDocument/object_test - ns/op 8822 ns/op 8560 ns/op 1.03
BenchmarkDocument/object_test - B/op 6817 B/op 6817 B/op 1
BenchmarkDocument/object_test - allocs/op 120 allocs/op 120 allocs/op 1
BenchmarkDocument/array_test 29894 ns/op 11947 B/op 276 allocs/op 29097 ns/op 11947 B/op 276 allocs/op 1.03
BenchmarkDocument/array_test - ns/op 29894 ns/op 29097 ns/op 1.03
BenchmarkDocument/array_test - B/op 11947 B/op 11947 B/op 1
BenchmarkDocument/array_test - allocs/op 276 allocs/op 276 allocs/op 1
BenchmarkDocument/text_test 31319 ns/op 14715 B/op 469 allocs/op 30508 ns/op 14718 B/op 469 allocs/op 1.03
BenchmarkDocument/text_test - ns/op 31319 ns/op 30508 ns/op 1.03
BenchmarkDocument/text_test - B/op 14715 B/op 14718 B/op 1.00
BenchmarkDocument/text_test - allocs/op 469 allocs/op 469 allocs/op 1
BenchmarkDocument/text_composition_test 29572 ns/op 18422 B/op 484 allocs/op 28706 ns/op 18420 B/op 484 allocs/op 1.03
BenchmarkDocument/text_composition_test - ns/op 29572 ns/op 28706 ns/op 1.03
BenchmarkDocument/text_composition_test - B/op 18422 B/op 18420 B/op 1.00
BenchmarkDocument/text_composition_test - allocs/op 484 allocs/op 484 allocs/op 1
BenchmarkDocument/rich_text_test 83522 ns/op 38476 B/op 1148 allocs/op 80269 ns/op 38476 B/op 1148 allocs/op 1.04
BenchmarkDocument/rich_text_test - ns/op 83522 ns/op 80269 ns/op 1.04
BenchmarkDocument/rich_text_test - B/op 38476 B/op 38476 B/op 1
BenchmarkDocument/rich_text_test - allocs/op 1148 allocs/op 1148 allocs/op 1
BenchmarkDocument/counter_test 18039 ns/op 10722 B/op 244 allocs/op 17241 ns/op 10722 B/op 244 allocs/op 1.05
BenchmarkDocument/counter_test - ns/op 18039 ns/op 17241 ns/op 1.05
BenchmarkDocument/counter_test - B/op 10722 B/op 10722 B/op 1
BenchmarkDocument/counter_test - allocs/op 244 allocs/op 244 allocs/op 1
BenchmarkDocument/text_edit_gc_100 1324468 ns/op 870942 B/op 16752 allocs/op 1273425 ns/op 870930 B/op 16752 allocs/op 1.04
BenchmarkDocument/text_edit_gc_100 - ns/op 1324468 ns/op 1273425 ns/op 1.04
BenchmarkDocument/text_edit_gc_100 - B/op 870942 B/op 870930 B/op 1.00
BenchmarkDocument/text_edit_gc_100 - allocs/op 16752 allocs/op 16752 allocs/op 1
BenchmarkDocument/text_edit_gc_1000 52382640 ns/op 50536338 B/op 181715 allocs/op 49717619 ns/op 50535600 B/op 181715 allocs/op 1.05
BenchmarkDocument/text_edit_gc_1000 - ns/op 52382640 ns/op 49717619 ns/op 1.05
BenchmarkDocument/text_edit_gc_1000 - B/op 50536338 B/op 50535600 B/op 1.00
BenchmarkDocument/text_edit_gc_1000 - allocs/op 181715 allocs/op 181715 allocs/op 1
BenchmarkDocument/text_split_gc_100 1887523 ns/op 1528754 B/op 15604 allocs/op 1846941 ns/op 1528848 B/op 15605 allocs/op 1.02
BenchmarkDocument/text_split_gc_100 - ns/op 1887523 ns/op 1846941 ns/op 1.02
BenchmarkDocument/text_split_gc_100 - B/op 1528754 B/op 1528848 B/op 1.00
BenchmarkDocument/text_split_gc_100 - allocs/op 15604 allocs/op 15605 allocs/op 1.00
BenchmarkDocument/text_split_gc_1000 115286987 ns/op 135077648 B/op 182203 allocs/op 110131529 ns/op 135077427 B/op 182189 allocs/op 1.05
BenchmarkDocument/text_split_gc_1000 - ns/op 115286987 ns/op 110131529 ns/op 1.05
BenchmarkDocument/text_split_gc_1000 - B/op 135077648 B/op 135077427 B/op 1.00
BenchmarkDocument/text_split_gc_1000 - allocs/op 182203 allocs/op 182189 allocs/op 1.00
BenchmarkDocument/text_delete_all_10000 19787072 ns/op 10183789 B/op 40676 allocs/op 15308841 ns/op 10184323 B/op 40677 allocs/op 1.29
BenchmarkDocument/text_delete_all_10000 - ns/op 19787072 ns/op 15308841 ns/op 1.29
BenchmarkDocument/text_delete_all_10000 - B/op 10183789 B/op 10184323 B/op 1.00
BenchmarkDocument/text_delete_all_10000 - allocs/op 40676 allocs/op 40677 allocs/op 1.00
BenchmarkDocument/text_delete_all_100000 338100617 ns/op 142712506 B/op 411780 allocs/op 254599250 ns/op 142662684 B/op 411654 allocs/op 1.33
BenchmarkDocument/text_delete_all_100000 - ns/op 338100617 ns/op 254599250 ns/op 1.33
BenchmarkDocument/text_delete_all_100000 - B/op 142712506 B/op 142662684 B/op 1.00
BenchmarkDocument/text_delete_all_100000 - allocs/op 411780 allocs/op 411654 allocs/op 1.00
BenchmarkDocument/text_100 223915 ns/op 120035 B/op 5081 allocs/op 224702 ns/op 120037 B/op 5081 allocs/op 1.00
BenchmarkDocument/text_100 - ns/op 223915 ns/op 224702 ns/op 1.00
BenchmarkDocument/text_100 - B/op 120035 B/op 120037 B/op 1.00
BenchmarkDocument/text_100 - allocs/op 5081 allocs/op 5081 allocs/op 1
BenchmarkDocument/text_1000 2391817 ns/op 1169025 B/op 50085 allocs/op 2441242 ns/op 1169025 B/op 50085 allocs/op 0.98
BenchmarkDocument/text_1000 - ns/op 2391817 ns/op 2441242 ns/op 0.98
BenchmarkDocument/text_1000 - B/op 1169025 B/op 1169025 B/op 1
BenchmarkDocument/text_1000 - allocs/op 50085 allocs/op 50085 allocs/op 1
BenchmarkDocument/array_1000 1226716 ns/op 1091441 B/op 11832 allocs/op 1258181 ns/op 1091308 B/op 11831 allocs/op 0.97
BenchmarkDocument/array_1000 - ns/op 1226716 ns/op 1258181 ns/op 0.97
BenchmarkDocument/array_1000 - B/op 1091441 B/op 1091308 B/op 1.00
BenchmarkDocument/array_1000 - allocs/op 11832 allocs/op 11831 allocs/op 1.00
BenchmarkDocument/array_10000 13899606 ns/op 9799434 B/op 120294 allocs/op 13046422 ns/op 9800304 B/op 120298 allocs/op 1.07
BenchmarkDocument/array_10000 - ns/op 13899606 ns/op 13046422 ns/op 1.07
BenchmarkDocument/array_10000 - B/op 9799434 B/op 9800304 B/op 1.00
BenchmarkDocument/array_10000 - allocs/op 120294 allocs/op 120298 allocs/op 1.00
BenchmarkDocument/array_gc_100 153657 ns/op 132716 B/op 1260 allocs/op 156613 ns/op 132721 B/op 1260 allocs/op 0.98
BenchmarkDocument/array_gc_100 - ns/op 153657 ns/op 156613 ns/op 0.98
BenchmarkDocument/array_gc_100 - B/op 132716 B/op 132721 B/op 1.00
BenchmarkDocument/array_gc_100 - allocs/op 1260 allocs/op 1260 allocs/op 1
BenchmarkDocument/array_gc_1000 1440793 ns/op 1159216 B/op 12877 allocs/op 1447385 ns/op 1159129 B/op 12876 allocs/op 1.00
BenchmarkDocument/array_gc_1000 - ns/op 1440793 ns/op 1447385 ns/op 1.00
BenchmarkDocument/array_gc_1000 - B/op 1159216 B/op 1159129 B/op 1.00
BenchmarkDocument/array_gc_1000 - allocs/op 12877 allocs/op 12876 allocs/op 1.00
BenchmarkDocument/counter_1000 204759 ns/op 193081 B/op 5771 allocs/op 210478 ns/op 193080 B/op 5771 allocs/op 0.97
BenchmarkDocument/counter_1000 - ns/op 204759 ns/op 210478 ns/op 0.97
BenchmarkDocument/counter_1000 - B/op 193081 B/op 193080 B/op 1.00
BenchmarkDocument/counter_1000 - allocs/op 5771 allocs/op 5771 allocs/op 1
BenchmarkDocument/counter_10000 2218620 ns/op 2088012 B/op 59778 allocs/op 2197933 ns/op 2087995 B/op 59778 allocs/op 1.01
BenchmarkDocument/counter_10000 - ns/op 2218620 ns/op 2197933 ns/op 1.01
BenchmarkDocument/counter_10000 - B/op 2088012 B/op 2087995 B/op 1.00
BenchmarkDocument/counter_10000 - allocs/op 59778 allocs/op 59778 allocs/op 1
BenchmarkDocument/object_1000 1413637 ns/op 1428005 B/op 9849 allocs/op 1454484 ns/op 1428281 B/op 9849 allocs/op 0.97
BenchmarkDocument/object_1000 - ns/op 1413637 ns/op 1454484 ns/op 0.97
BenchmarkDocument/object_1000 - B/op 1428005 B/op 1428281 B/op 1.00
BenchmarkDocument/object_1000 - allocs/op 9849 allocs/op 9849 allocs/op 1
BenchmarkDocument/object_10000 16041002 ns/op 12166622 B/op 100565 allocs/op 15301284 ns/op 12166600 B/op 100564 allocs/op 1.05
BenchmarkDocument/object_10000 - ns/op 16041002 ns/op 15301284 ns/op 1.05
BenchmarkDocument/object_10000 - B/op 12166622 B/op 12166600 B/op 1.00
BenchmarkDocument/object_10000 - allocs/op 100565 allocs/op 100564 allocs/op 1.00
BenchmarkDocument/tree_100 1085553 ns/op 943702 B/op 6101 allocs/op 1062287 ns/op 943703 B/op 6101 allocs/op 1.02
BenchmarkDocument/tree_100 - ns/op 1085553 ns/op 1062287 ns/op 1.02
BenchmarkDocument/tree_100 - B/op 943702 B/op 943703 B/op 1.00
BenchmarkDocument/tree_100 - allocs/op 6101 allocs/op 6101 allocs/op 1
BenchmarkDocument/tree_1000 81877514 ns/op 86461062 B/op 60115 allocs/op 77961120 ns/op 86460497 B/op 60115 allocs/op 1.05
BenchmarkDocument/tree_1000 - ns/op 81877514 ns/op 77961120 ns/op 1.05
BenchmarkDocument/tree_1000 - B/op 86461062 B/op 86460497 B/op 1.00
BenchmarkDocument/tree_1000 - allocs/op 60115 allocs/op 60115 allocs/op 1
BenchmarkDocument/tree_10000 10061901247 ns/op 8580651104 B/op 600210 allocs/op 9697504238 ns/op 8580653984 B/op 600228 allocs/op 1.04
BenchmarkDocument/tree_10000 - ns/op 10061901247 ns/op 9697504238 ns/op 1.04
BenchmarkDocument/tree_10000 - B/op 8580651104 B/op 8580653984 B/op 1.00
BenchmarkDocument/tree_10000 - allocs/op 600210 allocs/op 600228 allocs/op 1.00
BenchmarkDocument/tree_delete_all_1000 79172756 ns/op 87510109 B/op 75266 allocs/op 78919740 ns/op 87510061 B/op 75266 allocs/op 1.00
BenchmarkDocument/tree_delete_all_1000 - ns/op 79172756 ns/op 78919740 ns/op 1.00
BenchmarkDocument/tree_delete_all_1000 - B/op 87510109 B/op 87510061 B/op 1.00
BenchmarkDocument/tree_delete_all_1000 - allocs/op 75266 allocs/op 75266 allocs/op 1
BenchmarkDocument/tree_edit_gc_100 3918279 ns/op 4146682 B/op 15141 allocs/op 3909758 ns/op 4146635 B/op 15140 allocs/op 1.00
BenchmarkDocument/tree_edit_gc_100 - ns/op 3918279 ns/op 3909758 ns/op 1.00
BenchmarkDocument/tree_edit_gc_100 - B/op 4146682 B/op 4146635 B/op 1.00
BenchmarkDocument/tree_edit_gc_100 - allocs/op 15141 allocs/op 15140 allocs/op 1.00
BenchmarkDocument/tree_edit_gc_1000 319481792 ns/op 383745310 B/op 154849 allocs/op 314942165 ns/op 383743300 B/op 154856 allocs/op 1.01
BenchmarkDocument/tree_edit_gc_1000 - ns/op 319481792 ns/op 314942165 ns/op 1.01
BenchmarkDocument/tree_edit_gc_1000 - B/op 383745310 B/op 383743300 B/op 1.00
BenchmarkDocument/tree_edit_gc_1000 - allocs/op 154849 allocs/op 154856 allocs/op 1.00
BenchmarkDocument/tree_split_gc_100 2615753 ns/op 2412481 B/op 11125 allocs/op 2618040 ns/op 2412474 B/op 11125 allocs/op 1.00
BenchmarkDocument/tree_split_gc_100 - ns/op 2615753 ns/op 2618040 ns/op 1.00
BenchmarkDocument/tree_split_gc_100 - B/op 2412481 B/op 2412474 B/op 1.00
BenchmarkDocument/tree_split_gc_100 - allocs/op 11125 allocs/op 11125 allocs/op 1
BenchmarkDocument/tree_split_gc_1000 192509755 ns/op 222250930 B/op 121996 allocs/op 194406734 ns/op 222254945 B/op 122005 allocs/op 0.99
BenchmarkDocument/tree_split_gc_1000 - ns/op 192509755 ns/op 194406734 ns/op 0.99
BenchmarkDocument/tree_split_gc_1000 - B/op 222250930 B/op 222254945 B/op 1.00
BenchmarkDocument/tree_split_gc_1000 - allocs/op 121996 allocs/op 122005 allocs/op 1.00
BenchmarkRPC/client_to_server 391941138 ns/op 16965424 B/op 175381 allocs/op 375433012 ns/op 16958754 B/op 175366 allocs/op 1.04
BenchmarkRPC/client_to_server - ns/op 391941138 ns/op 375433012 ns/op 1.04
BenchmarkRPC/client_to_server - B/op 16965424 B/op 16958754 B/op 1.00
BenchmarkRPC/client_to_server - allocs/op 175381 allocs/op 175366 allocs/op 1.00
BenchmarkRPC/client_to_client_via_server 654081242 ns/op 32505524 B/op 319964 allocs/op 627733928 ns/op 33433404 B/op 321037 allocs/op 1.04
BenchmarkRPC/client_to_client_via_server - ns/op 654081242 ns/op 627733928 ns/op 1.04
BenchmarkRPC/client_to_client_via_server - B/op 32505524 B/op 33433404 B/op 0.97
BenchmarkRPC/client_to_client_via_server - allocs/op 319964 allocs/op 321037 allocs/op 1.00
BenchmarkRPC/attach_large_document 1287805609 ns/op 1919079016 B/op 8851 allocs/op 1161544236 ns/op 1893869848 B/op 8792 allocs/op 1.11
BenchmarkRPC/attach_large_document - ns/op 1287805609 ns/op 1161544236 ns/op 1.11
BenchmarkRPC/attach_large_document - B/op 1919079016 B/op 1893869848 B/op 1.01
BenchmarkRPC/attach_large_document - allocs/op 8851 allocs/op 8792 allocs/op 1.01
BenchmarkRPC/adminCli_to_server 559735712 ns/op 35965996 B/op 289555 allocs/op 546518546 ns/op 35953332 B/op 289543 allocs/op 1.02
BenchmarkRPC/adminCli_to_server - ns/op 559735712 ns/op 546518546 ns/op 1.02
BenchmarkRPC/adminCli_to_server - B/op 35965996 B/op 35953332 B/op 1.00
BenchmarkRPC/adminCli_to_server - allocs/op 289555 allocs/op 289543 allocs/op 1.00
BenchmarkLocker 64.09 ns/op 16 B/op 1 allocs/op 64.02 ns/op 16 B/op 1 allocs/op 1.00
BenchmarkLocker - ns/op 64.09 ns/op 64.02 ns/op 1.00
BenchmarkLocker - B/op 16 B/op 16 B/op 1
BenchmarkLocker - allocs/op 1 allocs/op 1 allocs/op 1
BenchmarkLockerParallel 39.09 ns/op 0 B/op 0 allocs/op 37.94 ns/op 0 B/op 0 allocs/op 1.03
BenchmarkLockerParallel - ns/op 39.09 ns/op 37.94 ns/op 1.03
BenchmarkLockerParallel - B/op 0 B/op 0 B/op 1
BenchmarkLockerParallel - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkLockerMoreKeys 149.2 ns/op 15 B/op 0 allocs/op 154.5 ns/op 15 B/op 0 allocs/op 0.97
BenchmarkLockerMoreKeys - ns/op 149.2 ns/op 154.5 ns/op 0.97
BenchmarkLockerMoreKeys - B/op 15 B/op 15 B/op 1
BenchmarkLockerMoreKeys - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkChange/Push_10_Changes 4017251 ns/op 120992 B/op 1279 allocs/op 3926905 ns/op 121553 B/op 1284 allocs/op 1.02
BenchmarkChange/Push_10_Changes - ns/op 4017251 ns/op 3926905 ns/op 1.02
BenchmarkChange/Push_10_Changes - B/op 120992 B/op 121553 B/op 1.00
BenchmarkChange/Push_10_Changes - allocs/op 1279 allocs/op 1284 allocs/op 1.00
BenchmarkChange/Push_100_Changes 15080725 ns/op 567674 B/op 6648 allocs/op 14633073 ns/op 570141 B/op 6653 allocs/op 1.03
BenchmarkChange/Push_100_Changes - ns/op 15080725 ns/op 14633073 ns/op 1.03
BenchmarkChange/Push_100_Changes - B/op 567674 B/op 570141 B/op 1.00
BenchmarkChange/Push_100_Changes - allocs/op 6648 allocs/op 6653 allocs/op 1.00
BenchmarkChange/Push_1000_Changes 119945347 ns/op 5360468 B/op 63145 allocs/op 117188553 ns/op 5304217 B/op 63148 allocs/op 1.02
BenchmarkChange/Push_1000_Changes - ns/op 119945347 ns/op 117188553 ns/op 1.02
BenchmarkChange/Push_1000_Changes - B/op 5360468 B/op 5304217 B/op 1.01
BenchmarkChange/Push_1000_Changes - allocs/op 63145 allocs/op 63148 allocs/op 1.00
BenchmarkChange/Pull_10_Changes 3028067 ns/op 100059 B/op 1005 allocs/op 2905811 ns/op 101044 B/op 1004 allocs/op 1.04
BenchmarkChange/Pull_10_Changes - ns/op 3028067 ns/op 2905811 ns/op 1.04
BenchmarkChange/Pull_10_Changes - B/op 100059 B/op 101044 B/op 0.99
BenchmarkChange/Pull_10_Changes - allocs/op 1005 allocs/op 1004 allocs/op 1.00
BenchmarkChange/Pull_100_Changes 4606018 ns/op 264257 B/op 3475 allocs/op 4361143 ns/op 267085 B/op 3475 allocs/op 1.06
BenchmarkChange/Pull_100_Changes - ns/op 4606018 ns/op 4361143 ns/op 1.06
BenchmarkChange/Pull_100_Changes - B/op 264257 B/op 267085 B/op 0.99
BenchmarkChange/Pull_100_Changes - allocs/op 3475 allocs/op 3475 allocs/op 1
BenchmarkChange/Pull_1000_Changes 9323185 ns/op 1491270 B/op 29844 allocs/op 8638824 ns/op 1490949 B/op 29855 allocs/op 1.08
BenchmarkChange/Pull_1000_Changes - ns/op 9323185 ns/op 8638824 ns/op 1.08
BenchmarkChange/Pull_1000_Changes - B/op 1491270 B/op 1490949 B/op 1.00
BenchmarkChange/Pull_1000_Changes - allocs/op 29844 allocs/op 29855 allocs/op 1.00
BenchmarkSnapshot/Push_3KB_snapshot 17975268 ns/op 709220 B/op 6656 allocs/op 16973385 ns/op 714007 B/op 6654 allocs/op 1.06
BenchmarkSnapshot/Push_3KB_snapshot - ns/op 17975268 ns/op 16973385 ns/op 1.06
BenchmarkSnapshot/Push_3KB_snapshot - B/op 709220 B/op 714007 B/op 0.99
BenchmarkSnapshot/Push_3KB_snapshot - allocs/op 6656 allocs/op 6654 allocs/op 1.00
BenchmarkSnapshot/Push_30KB_snapshot 123917004 ns/op 5528960 B/op 63148 allocs/op 120494285 ns/op 5580394 B/op 63151 allocs/op 1.03
BenchmarkSnapshot/Push_30KB_snapshot - ns/op 123917004 ns/op 120494285 ns/op 1.03
BenchmarkSnapshot/Push_30KB_snapshot - B/op 5528960 B/op 5580394 B/op 0.99
BenchmarkSnapshot/Push_30KB_snapshot - allocs/op 63148 allocs/op 63151 allocs/op 1.00
BenchmarkSnapshot/Pull_3KB_snapshot 6867324 ns/op 918604 B/op 15508 allocs/op 6482619 ns/op 923531 B/op 15510 allocs/op 1.06
BenchmarkSnapshot/Pull_3KB_snapshot - ns/op 6867324 ns/op 6482619 ns/op 1.06
BenchmarkSnapshot/Pull_3KB_snapshot - B/op 918604 B/op 923531 B/op 0.99
BenchmarkSnapshot/Pull_3KB_snapshot - allocs/op 15508 allocs/op 15510 allocs/op 1.00
BenchmarkSnapshot/Pull_30KB_snapshot 16521227 ns/op 7155351 B/op 150108 allocs/op 15617224 ns/op 7151155 B/op 150106 allocs/op 1.06
BenchmarkSnapshot/Pull_30KB_snapshot - ns/op 16521227 ns/op 15617224 ns/op 1.06
BenchmarkSnapshot/Pull_30KB_snapshot - B/op 7155351 B/op 7151155 B/op 1.00
BenchmarkSnapshot/Pull_30KB_snapshot - allocs/op 150108 allocs/op 150106 allocs/op 1.00
BenchmarkSync/memory_sync_10_test 8211 ns/op 1286 B/op 38 allocs/op 7805 ns/op 1286 B/op 38 allocs/op 1.05
BenchmarkSync/memory_sync_10_test - ns/op 8211 ns/op 7805 ns/op 1.05
BenchmarkSync/memory_sync_10_test - B/op 1286 B/op 1286 B/op 1
BenchmarkSync/memory_sync_10_test - allocs/op 38 allocs/op 38 allocs/op 1
BenchmarkSync/memory_sync_100_test 53531 ns/op 8741 B/op 279 allocs/op 48678 ns/op 9008 B/op 296 allocs/op 1.10
BenchmarkSync/memory_sync_100_test - ns/op 53531 ns/op 48678 ns/op 1.10
BenchmarkSync/memory_sync_100_test - B/op 8741 B/op 9008 B/op 0.97
BenchmarkSync/memory_sync_100_test - allocs/op 279 allocs/op 296 allocs/op 0.94
BenchmarkSync/memory_sync_1000_test 591652 ns/op 74270 B/op 2115 allocs/op 411944 ns/op 83490 B/op 2689 allocs/op 1.44
BenchmarkSync/memory_sync_1000_test - ns/op 591652 ns/op 411944 ns/op 1.44
BenchmarkSync/memory_sync_1000_test - B/op 74270 B/op 83490 B/op 0.89
BenchmarkSync/memory_sync_1000_test - allocs/op 2115 allocs/op 2689 allocs/op 0.79
BenchmarkSync/memory_sync_10000_test 8605649 ns/op 738459 B/op 20268 allocs/op 5445002 ns/op 777161 B/op 22717 allocs/op 1.58
BenchmarkSync/memory_sync_10000_test - ns/op 8605649 ns/op 5445002 ns/op 1.58
BenchmarkSync/memory_sync_10000_test - B/op 738459 B/op 777161 B/op 0.95
BenchmarkSync/memory_sync_10000_test - allocs/op 20268 allocs/op 22717 allocs/op 0.89
BenchmarkTextEditing 5716363172 ns/op 3901900064 B/op 18743067 allocs/op 5276861919 ns/op 3901896400 B/op 18743156 allocs/op 1.08
BenchmarkTextEditing - ns/op 5716363172 ns/op 5276861919 ns/op 1.08
BenchmarkTextEditing - B/op 3901900064 B/op 3901896400 B/op 1.00
BenchmarkTextEditing - allocs/op 18743067 allocs/op 18743156 allocs/op 1.00
BenchmarkTree/10000_vertices_to_protobuf 3794470 ns/op 6262996 B/op 70025 allocs/op 3561687 ns/op 6262969 B/op 70025 allocs/op 1.07
BenchmarkTree/10000_vertices_to_protobuf - ns/op 3794470 ns/op 3561687 ns/op 1.07
BenchmarkTree/10000_vertices_to_protobuf - B/op 6262996 B/op 6262969 B/op 1.00
BenchmarkTree/10000_vertices_to_protobuf - allocs/op 70025 allocs/op 70025 allocs/op 1
BenchmarkTree/10000_vertices_from_protobuf 168368559 ns/op 442171624 B/op 290038 allocs/op 154792338 ns/op 442172595 B/op 290039 allocs/op 1.09
BenchmarkTree/10000_vertices_from_protobuf - ns/op 168368559 ns/op 154792338 ns/op 1.09
BenchmarkTree/10000_vertices_from_protobuf - B/op 442171624 B/op 442172595 B/op 1.00
BenchmarkTree/10000_vertices_from_protobuf - allocs/op 290038 allocs/op 290039 allocs/op 1.00
BenchmarkTree/20000_vertices_to_protobuf 8207893 ns/op 12716990 B/op 140028 allocs/op 7686180 ns/op 12716976 B/op 140028 allocs/op 1.07
BenchmarkTree/20000_vertices_to_protobuf - ns/op 8207893 ns/op 7686180 ns/op 1.07
BenchmarkTree/20000_vertices_to_protobuf - B/op 12716990 B/op 12716976 B/op 1.00
BenchmarkTree/20000_vertices_to_protobuf - allocs/op 140028 allocs/op 140028 allocs/op 1
BenchmarkTree/20000_vertices_from_protobuf 722491694 ns/op 1697263848 B/op 580043 allocs/op 679464128 ns/op 1697272824 B/op 580092 allocs/op 1.06
BenchmarkTree/20000_vertices_from_protobuf - ns/op 722491694 ns/op 679464128 ns/op 1.06
BenchmarkTree/20000_vertices_from_protobuf - B/op 1697263848 B/op 1697272824 B/op 1.00
BenchmarkTree/20000_vertices_from_protobuf - allocs/op 580043 allocs/op 580092 allocs/op 1.00
BenchmarkTree/30000_vertices_to_protobuf 12693678 ns/op 19318412 B/op 210030 allocs/op 11962644 ns/op 19318320 B/op 210030 allocs/op 1.06
BenchmarkTree/30000_vertices_to_protobuf - ns/op 12693678 ns/op 11962644 ns/op 1.06
BenchmarkTree/30000_vertices_to_protobuf - B/op 19318412 B/op 19318320 B/op 1.00
BenchmarkTree/30000_vertices_to_protobuf - allocs/op 210030 allocs/op 210030 allocs/op 1
BenchmarkTree/30000_vertices_from_protobuf 1694921131 ns/op 3752044360 B/op 870049 allocs/op 1606753074 ns/op 3752036416 B/op 870051 allocs/op 1.05
BenchmarkTree/30000_vertices_from_protobuf - ns/op 1694921131 ns/op 1606753074 ns/op 1.05
BenchmarkTree/30000_vertices_from_protobuf - B/op 3752044360 B/op 3752036416 B/op 1.00
BenchmarkTree/30000_vertices_from_protobuf - allocs/op 870049 allocs/op 870051 allocs/op 1.00

This comment was automatically generated by workflow using github-action-benchmark.

@codecov Codecov
Copy link

codecov bot commented Aug 14, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.06%. Comparing base (80c6ea0) to head (57ea87b).
Report is 2 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #964   +/-   ##
=======================================
  Coverage   51.06%   51.06%           
=======================================
  Files          73       73           
  Lines       10782    10782           
=======================================
  Hits         5506     5506           
  Misses       4725     4725           
  Partials      551      551

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@krapie krapie marked this pull request as draft August 14, 2024 14:57
@krapie
Copy link
Member

krapie commented Aug 14, 2024

@binary-ho If you are still working on the PR and it is not ready for the review, I recommend to create PR in draft mode.

@binary-ho binary-ho marked this pull request as ready for review August 14, 2024 17:02
@binary-ho binary-ho requested a review from krapie August 14, 2024 17:03
@krapie krapie added the enhancement 🌟 New feature or request label Aug 15, 2024
krapie
krapie requested changes Aug 15, 2024
View reviewed changes
Copy link
Member

@krapie krapie left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution.
I have left a few comments below.

For your questions:

Should the following packages be included or excluded in the benchmark targets?

It will be good to refer to the imported packages in the benchmark test codes. For example github.com/yorkie-team/yorkie/api/converter in push_pull_bench_test.go. converter package is responsible for converting between bytes and struct which will impact the benchmark test on client-server scenario.

That being said, let's include all the packages used in bench test including yorkie/client or yorkie/server/packs then shrink it as we monitor the CI usage.

.github/workflows/ci.yml Outdated
external-data-json-path: ./cache/benchmark-data.json
fail-on-alert: false
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-always: true

sharding_test:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also change the name sharding_test to sharding-test for consistency with other naming conventions?

.github/workflows/ci.yml Outdated
fail-on-alert: false
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-always: true
- name: Check Benchmark Target
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for checking changes in here also?
It seems like this job already passed this check in ci-target-check.

Copy link
Contributor Author

@binary-ho binary-ho Aug 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sorry to not checking carefully. I will delete it.

coderabbitai[bot]
coderabbitai bot reviewed Aug 15, 2024
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between df19964 and cdd85ad.

Files selected for processing (1)
  • .github/workflows/ci.yml (5 hunks)
Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/ci.yml

@binary-ho
Copy link
Contributor Author

binary-ho commented Aug 15, 2024 •
edited
Loading

thank you for review kevin
As discussed, I have organized the dependencies for the bench tests.
As a result, the benchmark tests will be triggered by changes in the following packages.
Could you please check this?

  bench: 
    - 'admin/**'
    - 'api/converter/**'
    - 'api/types/**'  
    - 'client/**'
    - 'pkg/document/**'
    - 'pkg/locker/**'
    - 'server/**'
    - 'test/helper/**'

1. Categorized by Dependency Package

1.1 Document

  • "pkg/document"
    • "pkg/document/json"
    • "pkg/document/presence"
    • "pkg/document/change"
    • "pkg/document/key"
    • "pkg/document/time"
    • "pkg/document/crdt"
  • "pkg/locker"

1.2 Server

  • "server"
    • "server/backend"
    • "server/backend/database"
    • "server/backend/sync"
    • "server/documents"
    • "server/logging"
    • "server/packs"
    • "server/profiling"
    • "server/profiling/prometheus"
    • "server/clients"
    • "server/projects"
    • "server/rpc"

1.3 API

  • "api/types"
  • "api/converter"

1.4 Client

  • "client"

1.5 Test

  • "test/helper"

2. Dependencies Organized by Test

(For reference)

  • document_bench_test

    • "pkg/document"

  • grpc_bench_test

    • "admin"
    • "api/types"
    • "client"
    • "pkg/document"
    • "pkg/document/json"
    • "pkg/document/presence"
    • "server"
    • "server/backend/database"
    • "server/logging"
    • "test/helper"

  • locker_bench_test

    • "pkg/locker"

  • push_pull_bench_test

    • "api/converter"
    • "api/types"
    • "pkg/document"
    • "pkg/document/change"
    • "pkg/document/json"
    • "pkg/document/key"
    • "pkg/document/presence"
    • "pkg/document/time"
    • "server/backend"
    • "server/backend/database"
    • "server/documents"
    • "server/packs"
    • "server/profiling/prometheus"
    • "test/helper"

  • sync_bench_test

    • "server/backend/sync"

  • text_editing_bench_test

    • "api/types"
    • "client"
    • "server/backend"
    • "server/backend/database"
    • "server/clients"
    • "server/profiling"
    • "server/profiling/prometheus"
    • "server/projects"
    • "server/rpc"

  • tree_editing_bench_test

    • "api/converter"
    • "pkg/document/crdt"
    • "pkg/document/json"
    • "test/helper"

@binary-ho binary-ho requested a review from krapie August 15, 2024 07:46
krapie
krapie reviewed Aug 15, 2024
View reviewed changes
Copy link
Member

@krapie krapie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the quick and detailed reflect.
One last suggestion before the merge :)

.github/workflows/ci.yml
with:
filters: |
build: '**'
bench:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about simplifying below packages like this?

            - 'pkg/**'
            - 'server/**'
            - 'client/**'
            - 'admin/**'
            - 'api/converter/**'

For api/types/** and /test/helper/**, we can skip it because it has low impact on the bench.
Also simplifying to pkg/** because this package is all about CRDT itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your quick review!

I was a bit concerned that I might have defined the bench target too narrowly,
but I think your suggestion is much better!

@krapie krapie self-requested a review August 15, 2024 10:49
coderabbitai[bot]
coderabbitai bot reviewed Aug 15, 2024
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between cdd85ad and 57ea87b.

Files selected for processing (1)
  • .github/workflows/ci.yml (5 hunks)
Additional comments not posted (4)
.github/workflows/ci.yml (4)

Line range hint 49-90:
Ensure build conditions and steps are necessary.

The build job is conditioned on changes detected by ci-target-check. Verify that each step is essential and correctly ordered, especially the Test and Upload coverage to Codecov steps, to ensure efficient CI execution.

Verification successful

CI Workflow Steps Are Correctly Configured

The build job in the CI workflow is correctly configured with the necessary Makefile targets (tools, lint, and build). These steps are essential for the build process and are correctly ordered. No issues were found with the current setup.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the necessity and order of steps in the `build` job.

# Test: Check if the `make tools`, `make lint`, and `make build` commands are defined in the Makefile.
# Expect: Each command should be present and correctly defined.
rg 'make (tools|lint|build)' Makefile

Length of output: 39

Script:

#!/bin/bash
# Check if a Makefile exists and list its contents to verify the presence of necessary targets.
fd -IH Makefile --exec cat

Length of output: 3153

Line range hint 96-135:
Validate benchmark conditions and steps.

The bench job is conditioned on changes detected by ci-target-check. Ensure that the steps, especially those involving caching and storing benchmark results, are correctly configured and necessary for the intended benchmarking process.

Verification successful

Benchmarking steps are correctly configured.

The bench target is defined in the Makefile, confirming that the benchmarking steps in the CI workflow are set up correctly. The make bench command should function as intended. No further action is needed regarding the benchmark configuration.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the benchmark steps in the `bench` job.

# Test: Check if the `make bench` command is defined and correctly configured in the Makefile.
# Expect: The command should be present and correctly defined.
rg 'make bench' Makefile

Length of output: 26

Script:

#!/bin/bash
# Check if the Makefile exists in the repository
fd -e Makefile

# Search for the definition of the `bench` target in the Makefile
rg '^bench:' Makefile

Length of output: 71

20-44: Verify path filters and outputs.

The ci-target-check job uses dorny/paths-filter to set conditions for subsequent jobs. Ensure that the paths specified for bench and sharding-test accurately reflect the intended scope of changes. This setup is crucial for optimizing CI runs by targeting only necessary jobs.

Verification successful

Path filters in ci-target-check are correctly configured.

The paths specified for both bench and sharding-test filters in the ci-target-check job accurately reflect the intended directories. This setup ensures efficient CI targeting based on code changes.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the paths specified in the `ci-target-check` job are accurate.

# Test: List all paths under `pkg`, `server`, `client`, `admin`, and `api/converter`.
# Expect: Paths should match the intended scope for benchmarks and sharding tests.
fd --type d --max-depth 2 --exec echo "Path: {}"

Length of output: 819

Script:

#!/bin/bash
# Description: Check for the existence of the `server/backend/database` directory.

# Test: Verify if `server/backend/database` exists.
fd --type d --exact-depth 3 --exec echo "Path: {}" | grep 'server/backend/database'

Length of output: 115

Line range hint 140-182:
Review sharding-test conditions and Docker steps.

The sharding-test job is conditioned on changes detected by ci-target-check. Ensure that all Docker Compose steps, including initialization scripts, are necessary and executed in the correct order for successful sharding tests.

Verification successful

Verification successful: Necessary Docker Compose files and scripts are present.

The sharding-test job in the CI workflow has all the required Docker Compose files and initialization scripts available in the repository. This supports the setup described in the review comment, indicating that the job is correctly configured in terms of file dependencies.

  • Docker Compose File: ./build/docker/sharding/docker-compose.yml
  • Initialization Scripts:
    • ./build/docker/sharding/scripts/init-config1.js
    • ./build/docker/sharding/scripts/init-shard1-1.js
    • ./build/docker/sharding/scripts/init-shard2-1.js
    • ./build/docker/sharding/scripts/init-mongos1.js
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Docker Compose steps in the `sharding-test` job.

# Test: Check the presence and correctness of Docker Compose files and scripts.
# Expect: Files and scripts should exist and be correctly configured.
fd --type f --name 'docker-compose.yml' --exec echo "Docker Compose File: {}"
fd --type f --name '*.js' --exec echo "Script File: {}"

Length of output: 637

Script:

#!/bin/bash
# Corrected script to verify Docker Compose steps in the `sharding-test` job.

# Test: Check the presence of Docker Compose files and initialization scripts.
# Expect: Files and scripts should exist.
fd 'docker-compose.yml' --type f --exec echo "Docker Compose File: {}"
fd '.js' --type f --exec echo "Script File: {}"

Length of output: 528

krapie
krapie approved these changes Aug 15, 2024
View reviewed changes
Copy link
Member

@krapie krapie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏼

@krapie krapie merged commit 175d53b into main Aug 15, 2024
5 checks passed
@krapie krapie deleted the ci-fine-tune branch August 15, 2024 10:55
@hackerwins
Copy link
Member

@krapie , @binary-ho

For some reason the CI was broken in main. Now I've rolled back the main branch to the previous commit.

https://github.com/yorkie-team/yorkie/actions/runs/10402716854

@krapie krapie restored the ci-fine-tune branch August 16, 2024 03:07
@krapie krapie mentioned this pull request Aug 16, 2024
Merged
2 tasks
@binary-ho
Copy link
Contributor Author

binary-ho commented Aug 16, 2024 •
edited
Loading

The Reason build failed is that the checkout step was missing in the ci-target-check job.


The following step should be added:

   - name: Check out code
    uses: actions/checkout@v4

The ci-target-check job should be updated as follows:

jobs:
  ci-target-check:
  
    ...
    steps:
    - name: Check out code
    uses: actions/checkout@v4

    - name: CI target check by path
      uses: dorny/paths-filter@v3
      id: ci-target-check
      with:
        filters: |
          build: '**'
          bench: 
            - 'pkg/**'
            - 'server/**'
            - 'client/**'
            - 'admin/**'
            - 'api/converter/**'
          
          sharding-test:
            - 'server/backend/database/**'

The reason why the PR build succeeded was that “dorny/paths-filter@v3” did not require a checkout in the PR.

image


I misunderstood this comment and removed it entirely.

This issue was found by @devleejb.
and @krapie has confirmed the issue and will take action to address it.

Thank you, devleejb and krapie

@krapie
Copy link
Member

krapie commented Aug 16, 2024

@binary-ho I will merge the following PR: #965 soon.
Therefore, you don't have to take further actions :)

hackerwins pushed a commit that referenced this pull request Aug 16, 2024
@krapie @binary-ho
[Revised] Fine-tuned CI Workflows in PR (#965)
ed6e974 
Revised version of fine-tuned CI workflows in PR #964, which is
actions/checkout for dorny/paths-filter.

The CI failed in main branch, due to dorny/paths-filter action's
behavior. It does not require checkout when triggered by PR, but it
requires checkout on other triggers.

---------

Co-authored-by: binary_ho <dfghcvb11@naver.com>
@coderabbitai coderabbitai bot mentioned this pull request Sep 11, 2024
Merged
raararaara pushed a commit that referenced this pull request Oct 7, 2024
@krapie @binary-ho @raararaara
[Revised] Fine-tuned CI Workflows in PR (#965)
fa0be2d 
Revised version of fine-tuned CI workflows in PR #964, which is
actions/checkout for dorny/paths-filter.

The CI failed in main branch, due to dorny/paths-filter action's
behavior. It does not require checkout when triggered by PR, but it
requires checkout on other triggers.

---------

Co-authored-by: binary_ho <dfghcvb11@naver.com>
@binary-ho binary-ho mentioned this pull request Nov 1, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@github-actions github-actions[bot] github-actions[bot] left review comments

@krapie krapie krapie approved these changes

Assignees

@binary-ho binary-ho

Labels
enhancement 🌟 New feature or request
Projects
Yorkie Project - 2024
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fine-tune CI Workflows in PR
3 participants
@binary-ho @krapie @hackerwins