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!

Updated the commit message to the following:

| ci: conditionally install lua-cjson for perf job
|
| 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 works around it by installing the package only if it is not
| installed in the system. Also, it installs it only for Lua 5.1, which is
| compatible with LuaJIT, to avoid installation of modern Lua versions by
| default, since Lua 5.1 is not the default for most modern distros.
|
| Relates to tarantool/tarantool#12600

Branch is force-pushed.

On 29.04.26, Sergey Bronnikov wrote:
> Hi,
> 
> thanks for the comments. LGTM after updating a commit message.
> 
> Sergey
> 
> On 4/28/26 15:29, Sergey Kaplun wrote:
> > 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.
> Probably it is worth to add to the issue that the problem is affected 
> LuaJIT CI as well.

Added.

> >
> >> 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?
> Agree.
> >> 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 ...
> 
> Ok, let's rephrase like you suggested.
> 
> Also, it is worth referring to the issue #12600 ("See also #12600").

Rephrased.

> 
> >
> >>> 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?
> Yes, please.

Added.

> >
> >>>          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