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

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

[-- Attachment #1: Type: text/plain, Size: 4129 bytes --]

Hi, Sergey,

thanks! LGTM

(the job https://github.com/tarantool/luajit/actions/runs/25105723095 is 
failed,

so I restarted it)

Sergey

On 4/29/26 14:15, Sergey Kaplun wrote:
> 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
>>>

[-- Attachment #2: Type: text/html, Size: 7070 bytes --]