[Tarantool-patches] [PATCH v2 3/5] feedback_daemon: send feedback on server start

[Serge Petrenko]

10.04.2021 00:53, Vladislav Shpilevoy пишет:
> Hi! Thanks for the patchset!

Thanks for the review!

>
> On 08.04.2021 15:38, Serge Petrenko via Tarantool-patches wrote:
>> Send the first report as soon as instance's initial configuration
>> finishes.
>>
>> Part of #5750
>> ---
>>   src/box/lua/feedback_daemon.lua | 10 ++++++++++
>>   src/box/lua/load_cfg.lua        | 34 ++++++++++++++++++++++-----------
>>   2 files changed, 33 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
>> index d00eedf39..43bbc1fa2 100644
>> --- a/src/box/lua/feedback_daemon.lua
>> +++ b/src/box/lua/feedback_daemon.lua
>> @@ -323,6 +323,12 @@ local function fill_in_feedback(feedback)
>>       return feedback
>>   end
>>   
>> +-- fixme: remove this hack.
>> +-- It's here to prevent too early feedback sending.
>> +-- This leads to problems with thread sanitization after fork() on Mac OS.
>> +-- Google objc_initializeAfterForkError for details.
>> +local is_first_send = true
> Have you tried this?
> https://stackoverflow.com/questions/50168647/multiprocessing-causes-python-to-crash-and-gives-an-error-may-have-been-in-progr

Yes, I have. I was worried there are some test failures in CI caused by
curl's too early initialization, which wouldn't be solved by setting that
env variable. Actually there is such a problem in CI. I've tried a patch
with sleeping only on Mac, and release_asan_clang
job has a lot of failed tests, each reporting a leak from
CRYPTO_memdup/CRYPTO_strdup/CRYPTO_strndup:
https://github.com/tarantool/tarantool/runs/2313054777?check_suite_focus=true

Moreover, I'm afraid that setting that env variable we may miss any further
alerts from Mac's thread/fork sanitizer.

Long story short, let's leave this hack as is for now.

Here's the patch I was trying to apply:

diff --git a/src/box/lua/feedback_daemon.lua 
b/src/box/lua/feedback_daemon.lua
index 43bbc1fa2..cc4e24559 100644
--- a/src/box/lua/feedback_daemon.lua
+++ b/src/box/lua/feedback_daemon.lua
@@ -327,7 +327,7 @@ end
  -- It's here to prevent too early feedback sending.
  -- This leads to problems with thread sanitization after fork() on Mac OS.
  -- Google objc_initializeAfterForkError for details.
-local is_first_send = true
+local is_first_send = jit.os == 'OSX' and true or nil

  local function feedback_loop(self)
      fiber.name(PREFIX, { truncate = true })
>
> We could add it to the CI jobs so the env variable would be
> set. And set it manually on our own machines.
>
> You can also use jit.os to start it without a delay on
> non-Mac machines. On Mac jit.os == 'OSX'. I see we use it
> in some places, so it looks legal and working.

-- 
Serge Petrenko