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.

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

refactor(build): remove 'rdsn' from build and test commands #1099

Merged
merged 5 commits into from
Aug 12, 2022

Conversation

acelyc111
Copy link
Member

@acelyc111 acelyc111 commented Aug 3, 2022

#1053

This patch is planning to remove 'rdsn' from build and test commands. After doing this, the CI will last longer, so I have to the following work to reduce the effect:

  1. Refactor Cpp CI yaml:
    a. shorten names, it will be easier to read on Web
    b. split 'test' into 'build' and 'test'
    c. remove some self-hosted runner configs
    d. use actions/cache@v3 to speed up build staged
    e. remove changes check on rdsn directory
    f. make -j parameter with N cpus instead N/2
    g. use actions/upload-artifact@v3 and actions/download-artifact@v3 to pass test artifacts from build job to test job
    h. use matrix to run test parallely to reduce test last time, and make re-run cheaper (and also add some missing test cases in run.sh)
    i. remove LSAN since it's included by ASAN
  2. Cmake changes:
    a. unify rdsn and pegasus cmakelists
    b. add ccache detection and configs in cmake
    c. adjust includes and links
    d. make pegasus_geo_test can be run by shell which is the unify way
    e. remove src/rdsn/CMakeLists.txt
  3. shell scripts update
    a. remove 'rdsn' from run.sh for both build and test functions
    b. re-org thrift generating and git_commit.h generating
    c. use different build dirs for different build types, then you can switch build types with lower cost (NOTE: you have to remove src/builder directory after this patch)
    d. make wget more robust (by wget -T 10 -t 5) Pegasus unit test failed for download hadoop failed #1095
  4. unit test fixes after this patch because it will run more tests
    a. move templates to header files (src/rdsn/src/meta/meta_service.h)
    b. other cmake and cpp files

image

@acelyc111 acelyc111 closed this Aug 3, 2022
@acelyc111 acelyc111 reopened this Aug 3, 2022
@github-actions github-actions bot added the github label Aug 3, 2022
@acelyc111 acelyc111 changed the title init rm_rdsn_cmakelist refactor(build): remove 'rdsn' in build and test commands Aug 3, 2022
@github-actions github-actions bot added the cpp label Aug 4, 2022
@acelyc111 acelyc111 force-pushed the rm_rdsn_cmakelist branch 2 times, most recently from 87476e4 to 46f6bdc Compare August 8, 2022 23:33
@acelyc111 acelyc111 changed the title refactor(build): remove 'rdsn' in build and test commands refactor(build): remove 'rdsn' from build and test commands Aug 8, 2022
@acelyc111 acelyc111 marked this pull request as ready for review August 10, 2022 13:10
hycdong
hycdong previously approved these changes Aug 11, 2022
@hycdong
Copy link
Contributor

hycdong commented Aug 11, 2022

LGTM~

Besides, I have following suggestions:

  1. I recommend checking if pack_server.sh, pack_client.sh, pack_tools.sh can work well. In our previous build refactor, those script may have problems after build refactor.
  2. I recommend refactoring pegasus_function_test in future. Restrict with test time, some long-time function tests are not running if on_travis is set as true.

@acelyc111
Copy link
Member Author

acelyc111 commented Aug 11, 2022

  • I recommend checking if pack_server.sh, pack_client.sh, pack_tools.sh can work well. In our previous build refactor, those script may have problems after build refactor.
  • I recommend refactoring pegasus_function_test in future. Restrict with test time, some long-time function tests are not running if on_travis is set as true.
  1. Thanks for your reminding, I missed that, pack_server.sh and pack_tools.sh has been added back, pack_client.sh is not running before so I didn't add it this time, I will do it in another patch.
  2. Yes, I'll do it in another patch.

@acelyc111 acelyc111 merged commit ba98ada into apache:master Aug 12, 2022
ZhongChaoqiang added a commit to ZhongChaoqiang/incubator-pegasus that referenced this pull request Nov 28, 2022
@empiredan empiredan mentioned this pull request Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants