Alexander, thanks a lot for a deep review, I've answered the questions below.

Среда, 19 февраля 2020, 0:41 +03:00 от Alexander Turenko <alexander.turenko@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