Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add caution in system+semaphore #13035

Closed
wants to merge 6,555 commits into from
Closed

Add caution in system+semaphore #13035

wants to merge 6,555 commits into from

Conversation

@jderusse
Copy link
Contributor

jderusse commented Feb 3, 2020

Add a warning in documentation about using semaphore + systemd + non-system user

see symfony/symfony#29952 (comment)

javiereguiluz and others added 30 commits Jan 2, 2020
* 5.0:
  Update error_handler.rst
This PR was merged into the 4.3 branch.

Discussion
----------

Update argument_value_resolver.rst

Commits
-------

0e7a1ec Update argument_value_resolver.rst
* 4.3:
  Update argument_value_resolver.rst
* 4.4:
  Update argument_value_resolver.rst
* 5.0:
  Update argument_value_resolver.rst
…(atailouloute)

This PR was merged into the 5.0 branch.

Discussion
----------

Make custom routers compatible with the LoaderInterface

Commits
-------

a49af0d Make custom routers compatible with the LoaderInterface
* 5.0:
  Make custom routers compatible with the LoaderInterface
This PR was merged into the 4.3 branch.

Discussion
----------

Update routing.rst - Fix wrong XML

Commits
-------

3d0fed0 Update routing.rst - Fix wrong XML
This PR was merged into the 4.3 branch.

Discussion
----------

Update routing.rst

Commits
-------

085f5fb Update routing.rst
This PR was merged into the 4.3 branch.

Discussion
----------

Update routing.rst

Remove missing return

Commits
-------

2052995 Update routing.rst
This PR was submitted for the 5.0 branch but it was merged into the 4.3 branch instead (closes #12839).

Discussion
----------

Update event_dispatcher.rst

Commits
-------

20520f2 Update event_dispatcher.rst
* 4.3:
  Update event_dispatcher.rst
  Update routing.rst
* 4.4:
  Update event_dispatcher.rst
  Update routing.rst
* 5.0:
  Update event_dispatcher.rst
  Update routing.rst
Grammar.
This PR was submitted for the 5.0 branch but it was merged into the 4.3 branch instead (closes #12878).

Discussion
----------

Update doctrine.rst

Grammar.

Commits
-------

c23db39 Update doctrine.rst
* 4.3:
  Update doctrine.rst
* 4.4:
  Update doctrine.rst
* 5.0:
  Update doctrine.rst
* 3.4:
  Add a note about voters with access_control
* 4.3:
  Add a note about voters with access_control
* 4.4:
  Add a note about voters with access_control
* 5.0:
  Add a note about voters with access_control
* 3.4:
  Use HttpKernel Component
* 3.4:
  Use HttpKernel Component
* 4.3:
* 4.4:
  Use HttpKernel Component
* 5.0:
  Use HttpKernel Component
Following the documentation, we check the Calendar application and get the string "Yep, this is a leap year!" instead of "Hello Fabien".
wouterj added 9 commits Feb 1, 2020
* 5.0:
  [#12861] Tweaked translation docs for 4.4 component changes
  [#12861] Tweak translation articles for 4.3 component changes
* 4.3:
  Removed leftover 3.4 versionadded directive
* 4.4:
  Removed leftover 3.4 versionadded directive
* 5.0:
  Removed leftover 3.4 versionadded directive
* 3.4:
  Added the other two removed component articles to the redirection map
* 4.3:
  Added the other two removed component articles to the redirection map
* 4.4:
  Added the other two removed component articles to the redirection map
* 5.0:
  Added the other two removed component articles to the redirection map
@lyrixx
lyrixx approved these changes Feb 3, 2020
@hsibboni

This comment has been minimized.

Copy link

hsibboni commented Feb 4, 2020

Hello,
I don t think systemd consider this an issue. They closed it, saying :

Sorry, but this is nothing to fix upstream... Please ask the customer to disable RemoveIPC= locally, or fix his users to become proper system users.

Also, RemoveIPC=yes is the default value on some Linux distributions, I don't know what you mean by

default value for non-system user

I think it is a system wide parameter? (but I am not 100% sure of this).

Finally, I do not think it is related to cron, I just tested it with cron, and people in the original thread were all using a cron. So I wrote explanations for their cases. You just need to have a user that logs in and logs out for systemd to trigger the removal of IPC resources.

I hope it is clearer ?

I can try to reproduce it without a cron if you want.

@jderusse jderusse force-pushed the jderusse:lock-sem branch from c52f50c to d226b99 Feb 4, 2020
@jderusse

This comment has been minimized.

Copy link
Contributor Author

jderusse commented Feb 4, 2020

removed link to issue
reworded similar to what postgres says https://wiki.postgresql.org/wiki/Systemd

@javiereguiluz javiereguiluz added the Lock label Feb 4, 2020
Copy link
Member

HeahDude left a comment

Minor comments

@@ -631,6 +631,14 @@ can be two running containers in parallel.
concurrent process on a new machine, check that other process are stopped
on the old one.

.. caution::

When runing on systemd with non-system user with option ``RemoveIPC=yes``

This comment has been minimized.

Copy link
@HeahDude

HeahDude Feb 4, 2020

Member

with non-system user and option...?


When runing on systemd with non-system user with option ``RemoveIPC=yes``
(default value), locks are deleted by systemd when that user logs out.
Check that process is ran with a system user (UID <= SYS_UID_MAX) with

This comment has been minimized.

Copy link
@HeahDude

HeahDude Feb 4, 2020

Member
-ran
+run
@OskarStark OskarStark added this to the 3.4 milestone Feb 5, 2020
@OskarStark OskarStark changed the base branch from master to 3.4 Feb 5, 2020
OskarStark added a commit that referenced this pull request Feb 5, 2020
@OskarStark

This comment has been minimized.

Copy link
Contributor

OskarStark commented Feb 5, 2020

Unfortunately I had some problems merging this PR (as you can see in the Commits-Tab). I merged it manually b231871 and addressed @HeahDude's comments 5fcedb6

@OskarStark OskarStark closed this Feb 5, 2020
OskarStark added a commit that referenced this pull request Feb 5, 2020
* 3.4:
  minor. refs #13035
  Add caution in system+semaphore
OskarStark added a commit that referenced this pull request Feb 5, 2020
* 4.4:
  minor. refs #13035
  Add caution in system+semaphore
OskarStark added a commit that referenced this pull request Feb 5, 2020
* 5.0:
  minor. refs #13035
  Add caution in system+semaphore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.