Re: [Tarantool-patches] [PATCH luajit] ci: conditionally install lua-cjson for perf job

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey!
Thanks for the review!
Please consider my answers below.

On 28.04.26, Sergey Bronnikov wrote:
> Hi, Sergey!
> 
> A few things worry me: we still don't know the root cause of the problem.

It should be investigated and fixed in the scope of the 12600 [1]. For
LuaJIT's CI, this workaround works just fine.

> 
> The proposed patch merely reduces the frequency of module installations.

Yes, it installs the module only once.

> 
> And if an installation is required, the installation will still break.

Not always, it usually passes successfully and fails only on the fourth
run. So, 1 successful installation is enough to prevent flakiness, which
is OK for our needs in LuaJIT. It is better to have some statistics
instead of having none, isn't it?

> 
> See comments inline.
> 
> Sergey
> 
> On 4/23/26 20:08, Sergey Kaplun wrote:
> > We have encountered the flakiness of the network on our CI runners. It
> > happens only on lua-cjson installation and only in some particular job
> > run.
> >
> > This patch helps to deal with it by installing this package only if it
> s/helps to deal with it/helps to reduce a number of installations/

Don't get the comment about the number of installations. Maybe rephrase
it like the following?
| This patch workarounds it by installing the package only if ...

> > is not installed in the system. Also, it installs it only for Lua 5.1,
> > which is compatible with LuaJIT.
> > ---
> >
> > Branch:https://github.com/tarantool/luajit/tree/skaplun/ci-perf-install-conditionally-lua-cjson
> >
> >   .github/actions/setup-performance/action.yml | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/.github/actions/setup-performance/action.yml b/.github/actions/setup-performance/action.yml
> > index 4e0e1929..3c2a8230 100644
> > --- a/.github/actions/setup-performance/action.yml
> > +++ b/.github/actions/setup-performance/action.yml
> > @@ -11,7 +11,9 @@ runs:
> >           apt install -y curl luarocks util-linux
> >         shell: bash
> >       - name: Install Lua modules
> > -      run: luarocks install lua-cjson
> > +      run: >
> > +        luarocks --lua-version=5.1 show lua-cjson ||
> > +        luarocks --lua-version=5.1 install lua-cjson
> Why do we need specifying Lua version now? Previously, it worked without 
> setting exact version.

I've preferred to describe it specifically since the most modern distros
provide Lua 5.3+ by default. Should I add the comment in the commit
message?

> >         shell: bash
> >       - name: Run script to setup Linux environment
> >         run: sh ./perf/helpers/setup_env.sh

[1]: https://github.com/tarantool/tarantool/issues/12600

-- 
Best regards,
Sergey Kaplun