[Tarantool-patches] [PATCH v4] Implement perf testing at gitlab-ci
Alexander Tikhonov
avtikhon at tarantool.org
Wed Feb 19 08:37:13 MSK 2020
Alexander, thanks a lot for a deep review, I've answered the questions below.
>Среда, 19 февраля 2020, 0:41 +03:00 от Alexander Turenko <alexander.turenko at tarantool.org>:
>
>I still think that details might be better, but okay, I see: you need
>some base now to proceed further. Don't want to block it anymore.
Right, thanks a lot.
>
>
>I commented the patch below, but didn't perform any changes except a bit
>reworded commit message.
>
>Pushed to master. CCed Kirill.
>
>Don't sure how it should look at other release branches:
>
>- Whether something need to be changed for 2.3/2.2?
> - perf_only_template should have "2.3" / "2.2" branch instead of
> master, that I understood.
Right, that is the only change.
>
> - Should IMAGE_PERF be tagged as "perf_2.3" / "perf_2.2" instead of
> "perf_master"? Are benchmarks should be adjusted for those versions
> and should this lead to such separation base images (or will be
> handled at runtime with bench-run scripts?).
No, there is no any need to have separate IMAGE_PERF images, due to it
has only built benchmarks w/o depends to Tarantool sources.
>
>- Whether something need to be changed for 1.10?
> - At least SQL benchmarks will not work. Should it be handled here or
> they will be skipped on bench-run side?
> - Same question re IMAGE_PERF as above.
Right, it needs special Dockerfile w/o SQL benchmarks, I already have
separate 'avtikhon/gitlab-ci_1.10-perf' branch for it, I'll update it with changes
from the current commit.
>
>
>Let's elaborate those questions. After this we can push it downward.
>
>WBR, Alexander Turenko.
>
>> Implement perf testing at gitlab-ci
>
>Changed to: 'gitlab-ci: enable performance testing'.
Ok.
>
>
>>
>> Enabled Tarantool performance testing on Gitlab-CI
>> for release/master branches and "*-perf" named branches.
>> For this purpose 'perf' and 'cleanup' stages were added
>> into Gitlab-CI pipeline.
>>
>> Performance testing support next benchmarks:
>> - cbench
>> - linkbench
>> - nosqlbench (hash and tree Tarantool run modes)
>> - sysbench
>> - tpcc
>> - ycsb (hash and tree Tarantool run modes)
>>
>> Benchmarks use scripts from repository:
>> http://gitlab.com/tarantool/bench-run
>
>Dead link. Changed gitlab.com to github.com.
Right, thanks.
>
>
>>
>> Perfomance testing uses docker images, built
>
>Fixed typo: 'Perfomance'.
Right, thanks.
>
>> with docker files from bench-run repository:
>> - perf/ubuntu-bionic:perf_master
>> parent image with benchmarks only
>> - perf_tmp/ubuntu-bionic:perf_<commit_SHA>
>> child images used for testing Tarantool sources
>
>Formatted a bit (to fit 72 symbols, but not much less).
Ok.
>
>
>> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gitlab-ci-perf
>
>> +.perf_only_template: &perf_only_definition
>> + only:
>> + - master
>> + - /^.*-perf$/
>> + variables: &perf_vars_definition
>> + IMAGE_PERF: "${CI_REGISTRY}/${CI_PROJECT_PATH}/perf/ubuntu-bionic:perf_master"
>> + IMAGE_PERF_BUILT: "${CI_REGISTRY}/${CI_PROJECT_PATH}/perf_tmp/ubuntu-bionic:perf_${CI_COMMIT_SHORT_SHA}"
>> +
>
>The resulting bench-run API looks strage for me:
>
>* It expects that a caller will set CI_REGISTRY, CI_REGISTRY_USER,
> CI_REGISTRY_PASSWORD environment variables, which come from GitLab-CI,
> but can be set manually.
These variables are in use for docker registry login at bench-run scripts,
these variables recreating each job run by gitlab-ci.
>
>* However it does not use CI_REGISTRY, CI_PROJECT_PATH,
> CI_COMMIT_SHORT_SHA to choose images name on its own, but expect
> IMAGE_PERF and IMAGE_PERF_BUILT from a caller.
Gitlab-ci initiates in .gitlab-ci.yml jobs running under docker images, so right
here the performance image IMAGE_PERF_BUILT must be set, IMAGE_PERF
is the part of the image IMAGE_PERF_BUILT and it's setup better to have at the
same place as IMAGE_PERF image.
>
>* All those variables have prefix CI_*, not, say, BENCH_RUN_*.
CI_* are gitlab-ci variables and visible to the bench-run make targets - no need
to setup additional variables.
>
>
>> +# Pre-testing part
>> +
>> +perf_bootstrap:
>> + <<: *perf_only_definition
>> + stage: test
>> + tags:
>> + - perf
>> + script:
>> + - ${GITLAB_MAKE} perf_prepare
>
>There is no reason to use two terms for the same thing: bootstrap and
>prepare.
I'm ok with your suggestion, let's do it on the next commit iteration.
>
>
>Also I don't see a reason to extract such one-two-liners into a
>gitlab.mk.
Previously we decided not to use .gitlab-ci.yml for code and to use it from
standalone make files, anyway I'm Ok with your suggestion, let's discuss
it a bit.
>
>> +# Post-testing part
>> +
>> +remove_images:
>> + <<: *perf_only_definition
>> + stage: cleanup
>> + when: always
>> + tags:
>> + - perf
>> + script:
>> + - ${GITLAB_MAKE} perf_cleanup
>> +
>
>Same as above: there is no reason to name it both 'remove_images' and
>'perf_cleanup'.
Ok, the same answer as above.
--
Alexander Tikhonov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20200219/da95a84f/attachment.html>
More information about the Tarantool-patches
mailing list