From: Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, alexander.turenko@tarantool.org Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v2 3/5] feedback_daemon: send feedback on server start Date: Sat, 10 Apr 2021 18:03:35 +0300 [thread overview] Message-ID: <236018bf-5447-7909-25e7-d93bf667a9b0@tarantool.org> (raw) In-Reply-To: <3b023576-0a64-9347-e2cb-f355d6bd9e94@tarantool.org> 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
next prev parent reply other threads:[~2021-04-10 15:03 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-08 13:38 [Tarantool-patches] [PATCH v2 0/5] send feedback on start and on key events Serge Petrenko via Tarantool-patches 2021-04-08 13:38 ` [Tarantool-patches] [PATCH v2 1/5] feedback_daemon: include server uptime in the report Serge Petrenko via Tarantool-patches 2021-04-08 13:38 ` [Tarantool-patches] [PATCH v2 2/5] feedback_daemon: rename `send_test` to `send` Serge Petrenko via Tarantool-patches 2021-04-08 13:38 ` [Tarantool-patches] [PATCH v2 3/5] feedback_daemon: send feedback on server start Serge Petrenko via Tarantool-patches 2021-04-09 21:53 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-10 15:03 ` Serge Petrenko via Tarantool-patches [this message] 2021-04-08 13:38 ` [Tarantool-patches] [PATCH v2 4/5] feedback_daemon: generate report right before sending Serge Petrenko via Tarantool-patches 2021-04-09 21:53 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-10 15:03 ` Serge Petrenko via Tarantool-patches 2021-04-08 13:38 ` [Tarantool-patches] [PATCH v2 5/5] feedback_daemon: count and report some events Serge Petrenko via Tarantool-patches 2021-04-13 8:31 ` Alexander Turenko via Tarantool-patches 2021-04-13 10:38 ` Serge Petrenko via Tarantool-patches 2021-04-11 14:15 ` [Tarantool-patches] [PATCH v2 0/5] send feedback on start and on key events Vladislav Shpilevoy via Tarantool-patches 2021-04-12 6:04 ` Kirill Yukhin via Tarantool-patches 2021-04-13 9:44 ` Alexander Turenko via Tarantool-patches 2021-04-12 6:05 ` Kirill Yukhin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=236018bf-5447-7909-25e7-d93bf667a9b0@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=alexander.turenko@tarantool.org \ --cc=sergepetrenko@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2 3/5] feedback_daemon: send feedback on server start' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox