<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi,</p>
<p>thanks for the comments. LGTM after updating a commit message.</p>
<p>Sergey</p>
<div class="moz-cite-prefix">On 4/28/26 15:29, Sergey Kaplun wrote:<br>
</div>
<blockquote type="cite" cite="mid:afCoHnWTiL_vevdi@root">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
Thanks for the review!
Please consider my answers below.
On 28.04.26, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
A few things worry me: we still don't know the root cause of the problem.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
It should be investigated and fixed in the scope of the 12600 [1]. For
LuaJIT's CI, this workaround works just fine.</pre>
</blockquote>
Probably it is worth to add to the issue that the problem is
affected LuaJIT CI as well.
<blockquote type="cite" cite="mid:afCoHnWTiL_vevdi@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
The proposed patch merely reduces the frequency of module installations.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Yes, it installs the module only once.
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
And if an installation is required, the installation will still break.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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?
</pre>
</blockquote>
Agree.
<blockquote type="cite" cite="mid:afCoHnWTiL_vevdi@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
See comments inline.
Sergey
On 4/23/26 20:08, Sergey Kaplun wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">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
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">s/helps to deal with it/helps to reduce a number of installations/
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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 ...</pre>
</blockquote>
<p>Ok, let's rephrase like you suggested. </p>
<p>Also, it is worth referring to the issue #12600 ("See also
#12600").</p>
<blockquote type="cite" cite="mid:afCoHnWTiL_vevdi@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">is not installed in the system. Also, it installs it only for Lua 5.1,
which is compatible with LuaJIT.
---
<a class="moz-txt-link-freetext" href="Branch:https://github.com/tarantool/luajit/tree/skaplun/ci-perf-install-conditionally-lua-cjson">Branch:https://github.com/tarantool/luajit/tree/skaplun/ci-perf-install-conditionally-lua-cjson</a>
.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
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">Why do we need specifying Lua version now? Previously, it worked without
setting exact version.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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?</pre>
</blockquote>
Yes, please.
<blockquote type="cite" cite="mid:afCoHnWTiL_vevdi@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre"> shell: bash
- name: Run script to setup Linux environment
run: sh ./perf/helpers/setup_env.sh
</pre>
</blockquote>
</blockquote>
<pre wrap="" class="moz-quote-pre">
[1]: <a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/12600">https://github.com/tarantool/tarantool/issues/12600</a>
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>