devpiuser commented 7 years ago

The os.waitpid() call in isrunning() (server/devpi_server/vendor/xprocess.py) causes "devpi-server --status" to not work anymore, because waitpid() requires, that the waited-for process is a child process of the current process, which is not the case, so that a ChildProcessError (Python 3, subclass of OSError) or OSError (Python 2) is raised and isrunning() returns false, although the devpi-server process is running.

fschulze commented 7 years ago

I guess we have to detect whether the process is still a child and only call waitpid if that is the case.

theMarix commented 7 years ago

FYI: This also affects devpi-server --stop.

Thorbijoern commented 7 years ago

In my case I only have this problem when I have devpi-server with devpi-web installed. Somehow the problem does not occur, if I only have devpi-server installed.

konstunn commented 7 years ago

What temporary workaround can one use to start/stop, get status of the server? Manually ps, netstat, kill?

vlcinsky commented 7 years ago

temporary workaround (to stop the devpi-server), which worked for me is simple killall devpi-server.

I guess, systemd service unit (possibly restricted for --user) could do the work as it takes care to kill the processes started. The status by devpi-server would probably fail.

If I manage such unit configuration, I put it here.

konstunn commented 7 years ago

@vlcinsky, will devpi-server --gen-config do the trick?

vlcinsky commented 7 years ago

Workaround for broken devpi-server --start, devpi-server --status and devpi-server --stop using systemd unit.

Concept

do not manage devpi-server by calling devpi-server --start, --stop or --status, instead use user systemd unit
user systemd unit will be called devpi
user systemd unit will start devpi-server in foreground

Install devpi-server, stop it

Install somewhere devpi-server. In my case, I have created virtualenv managed by tox located at /home/javl/install/bin-tools/devpi/.tox/py36, use anything what works for you.

Make sure, the devpi-server process is stopped:

$ killall devpi-server

The devpi-server must be stopped before we will start managing it by systemd to prevent clashes.

Create `devpi.service` unit file

The file shall be located at ~/.config/systemd/user/devpi.service:

[Unit]
Description=Private pypi server by devpi-server

[Service]
ExecStart=/home/javl/install/bin-tools/devpi/.tox/py36/bin/devpi-server
Restart=always

[Install]
WantedBy=default.target

Modify the line ExecStart to point to your instance of devpi-server executable.

Enable and start the `devpi` service

We work with --user instance of systemd, so be sure to use --user with systemctl command.

Also, when refering to unit file, we cannot use -u, which refers to system unit, but must use --user-unit.

Systemd must learn about new unit files:

$ systemctl --user daemon-reload

Enable the devpi unit/service:

$ systemctl --user enable devpi.service

and start it:

$ systemclt --user start devpi.service

If you want your user units staring automatically after user login:

$ sudo loginctl enable-linger username

Commands to start/stop/status/log entries for devpi-server

To learn about status:

$ systemctl --user status devpi
● devpi.service - Private pypi server by devpi-server
   Loaded: loaded (/home/javl/.config/systemd/user/devpi.service; enabled; vendor preset: enabled)
   Active: active (running) since St 2017-07-19 10:59:34 CEST; 12min ago
 Main PID: 7707 (devpi-server)
   CGroup: /user.slice/user-1000.slice/user@1000.service/devpi.service
           └─7707 /home/javl/install/bin-tools/devpi/.tox/py36/bin/python3.6 /home/javl/install/bin-tools/devpi/.tox/py36/bin/devpi-server

čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,597 INFO  NOCTX devpi-server version: 4.3.0
čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,597 INFO  NOCTX serverdir: /home/javl/.devpi/server
čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,597 INFO  NOCTX uuid: bd46d9a0bbb04e0fa0be8aaae2818901
čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,597 INFO  NOCTX serving at url: http://localhost:3141 (might be http://[localhost]:3141 for IPv6)
čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,597 INFO  NOCTX using 50 threads
čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,598 INFO  NOCTX bug tracker: https://github.com/devpi/devpi/issues
čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,598 INFO  NOCTX IRC: #devpi on irc.freenode.net
čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,598 INFO  NOCTX Hit Ctrl-C to quit.
čec 19 10:59:37 T810 devpi-server[7707]: 2017-07-19 10:59:37,169 INFO  [req0] POST /root/pypi/
čec 19 10:59:37 T810 devpi-server[7707]: 2017-07-19 10:59:37,558 INFO  [req0] [Wtx31] setting projects cache for 'simplejson'

To stop it:

$ systemctl --user stop devpi

To start it:

$ systemctl --user start devpi

To see last log entries:

$ journalctl --user-unit devpi.service

-- Logs begin at Ne 2017-06-18 12:16:00 CEST, end at St 2017-07-19 11:10:50 CEST. -- čec 19 10:50:02 T810 systemd[2487]: Started Private pypi server by devpi-server. čec 19 10:50:02 T810 devpi-server[31392]: 2017-07-19 10:50:02,831 INFO NOCTX Loading node info from /home/javl/.devpi/server/.nodeinfo čec 19 10:50:02 T810 devpi-server[31392]: 2017-07-19 10:50:02,832 INFO NOCTX wrote nodeinfo to: /home/javl/.devpi/server/.nodeinfo ........ ........ čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,597 INFO NOCTX uuid: bd46d9a0bbb04e0fa0be8aaae2818901 čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,597 INFO NOCTX serving at url: http://localhost:3141 (might be http://[localhost]:3141 for IPv6) čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,597 INFO NOCTX using 50 threads čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,598 INFO NOCTX bug tracker: https://github.com/devpi/devpi/issues čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,598 INFO NOCTX IRC: #devpi on irc.freenode.net čec 19 10:59:35 T810 devpi-server[7707]: 2017-07-19 10:59:35,598 INFO NOCTX Hit Ctrl-C to quit. čec 19 10:59:37 T810 devpi-server[7707]: 2017-07-19 10:59:37,169 INFO [req0] POST /root/pypi/ čec 19 10:59:37 T810 devpi-server[7707]: 2017-07-19 10:59:37,558 INFO [req0] [Wtx31] setting projects cache for 'simplejson'

To follow log entries in style tail -f log-file.log use the -f:

$ journalctl --user-unit devpi.service -f

Summary

Provided solution functions in regards to start/stop/status properly not suffering from the problem with pid.
Small disadvantage is, the call is a bit longer
Advantage is, if the user unit is enabled, it will start automatically after next reboot when the user logs in.
journalctl and status provide much nicer access to information about how it work.
described solution could be handy even without devpi-server handling of pid files being broken.

vlcinsky commented 7 years ago

@konstunn thanks for the --gen-config hint, I did not use it yet.

Anyway, the unit generated by --gen-config would probably suffer from the problem with pid file as it uses forking.

Anyway, the Restart=always is handy addition, I edited my answer to include that line.

I guess, that systemd stops the process properly (sending it proper signal) to allow for cleanup, but I do not have time to investigate it further on.

Anyway, with systemd it is often better (because simpler) to use processes in foreground, forking becomes obsolete heritage as systemd takes care of managing processes.

fschulze commented 7 years ago

Yes, using a proper process manager which runs devpi-server in foreground is the best solution. I guess it makes sense to add that to the documentation and deprecate --start/--stop/--status. It's probably easier to update the docs with examples than maintaining xprocess.

vlcinsky commented 7 years ago

@fschulze deprecating --start/--stop/--status looks like efficient Occam's razor cut.

Anyway, there are some use cases, where we have to be careful not to cut too far:

quick start

Currently it is rather quick and easy to install and start using devpi-server.

Without the obsoleted options it would work even though it would require user to keep one open console window (or send it to background...).

To me it seems acceptable.

Installation on MS Windows

Could be probably simply managed by some Windows Service wrapper - an example would help.

Installation on Linux with System V style of process manager

Those who like System V style could take advantage of forking.

Anyway, using supervisord could be the way to go in such situation.

Summary

I like the idea of simplifying devpi-server by removing the xprochttps://en.wikipedia.org/wiki/Occam's_razoress stuff.

The only risk I see is worse perception of usability by users (where it currently plays quite well). Anyway, these risks do not seem to high and they can be mitigated by adding examples into doc.

toriningen commented 7 years ago

Until xprocess is properly deprecated, there is an alternative workaround that does not involve using external tools. Instead, you can monkey-patch os.waitpid to accept non-child process PIDs under certain conditions.

The patch I propose calls original os.waitpid, and if it raises ChildProcessError, then it checks if process exists. If it does, and os.WNOHANG flag is set, then wrapped waitpid would return successfully. Otherwise, it would raise ChildProcessError as it should have originally.

To apply this patch, create a file with .pth extension (name does not matter, could be devpi-waitpid-patch.pth for example) in your site-packages folder with this content:

import sys; exec("if sys.version_info.major == 2:\n\texec('def patch2():\\n\\timport errno\\n\\timport functools\\n\\timport os\\n\\n\\t@functools.wraps(os.waitpid)\\n\\tdef waitpid(pid, options):\\n\\t\\ttry:\\n\\t\\t\\treturn _waitpid(pid, options)\\n\\t\\texcept OSError as exc:\\n\\t\\t\\tif exc.errno != errno.ECHILD:\\n\\t\\t\\t\\traise\\n\\n\\t\\t\\tif not options & os.WNOHANG:\\n\\t\\t\\t\\traise\\n\\n\\t\\t\\ttry:\\n\\t\\t\\t\\tos.kill(pid, 0)\\n\\t\\t\\texcept OSError as exc2:\\n\\t\\t\\t\\tif exc2.errno != errno.ESRCH:\\n\\t\\t\\t\\t\\traise\\n\\n\\t\\t\\t\\traise exc\\n\\n\\t\\treturn (0, 0)\\n\\n\\t_waitpid = os.waitpid\\n\\tos.waitpid = waitpid\\n\\npatch2()\\n')\n")
import sys; exec("if sys.version_info.major == 3:\n\texec('def patch3():\\n\\timport functools\\n\\timport os\\n\\n\\t@functools.wraps(os.waitpid)\\n\\tdef waitpid(pid, options):\\n\\t\\ttry:\\n\\t\\t\\treturn _waitpid(pid, options)\\n\\t\\texcept ChildProcessError as exc:\\n\\t\\t\\tif not options & os.WNOHANG:\\n\\t\\t\\t\\traise\\n\\n\\t\\t\\ttry:\\n\\t\\t\\t\\tos.kill(pid, 0)\\n\\t\\t\\texcept ProcessLookupError as exc2:\\n\\t\\t\\t\\traise exc from exc2\\n\\n\\t\\treturn (0, 0)\\n\\n\\t_waitpid = os.waitpid\\n\\tos.waitpid = waitpid\\n\\npatch3()\\n')\n")

Make sure these are two lines exactly, and both start with "import ", otherwise it won't work.

In readable form, patch functions look like this:

# This is called for Python 2
def patch2():
    import errno
    import functools
    import os

    @functools.wraps(os.waitpid)
    def waitpid(pid, options):
        try:
            return _waitpid(pid, options)
        except OSError as exc:
            if exc.errno != errno.ECHILD:
                raise

            if not options & os.WNOHANG:
                raise

            try:
                os.kill(pid, 0)
            except OSError as exc2:
                if exc2.errno != errno.ESRCH:
                    raise

                raise exc

        return (0, 0)

    _waitpid = os.waitpid
    os.waitpid = waitpid

patch2()

# This is called for Python 3
def patch3():
    import functools
    import os

    @functools.wraps(os.waitpid)
    def waitpid(pid, options):
        try:
            return _waitpid(pid, options)
        except ChildProcessError as exc:
            if not options & os.WNOHANG:
                raise

            try:
                os.kill(pid, 0)
            except ProcessLookupError as exc2:
                raise exc from exc2

        return (0, 0)

    _waitpid = os.waitpid
    os.waitpid = waitpid

patch3()

I have tested this code to work with Python 2.7.13 and Python 3.6.1, but it's likely to work on earlier Python versions as well. Just keep in mind that this is temporary quickfix, and not a longterm production solution.

P.S. My sincere apologies to those receiving multiple email notifications because of me altering my comment — there were bugs that had to be fixed for this to serve as a useful comment.

fschulze commented 7 years ago

Fixed in #464. But I marked it as deprecated now. We need to expand the docs.

lhupfeldt commented 6 years ago

The --status and --stop options are still not working in 4.4. (Maybe I misunderstand what is fixed in #464 ) We are running devpi on redhat 6.8, and it could be a while before moving to 7.x (out of my hands). That means no systemd. I would like to wote for vote for the fixing and retaining the --status and --stop options for quite a while. Major version system update in the corporate world can be really slow.

fschulze commented 6 years ago

@lhupfeldt Could you give more details? Error messages?

I think redhat 6.8 has the supervisord package which can be used instead of systemd. Or you use an init script. Our plan is to document those options. Any help with that is greatly appreciated. For a production server those options have the additional benefit, that they start devpi after reboot.

fschulze commented 5 years ago

For devpi-server 5.0.0 the process management is more prominently deprecated and I expanded the --gen-config examples.

vlcinsky commented 5 years ago

Thanks @fschulze : As can be seen in the code, the systemd devpi.service unit is now using simple type of service. As a bonus we will be able to follow log entries using journalctl now (assuming default systemd configuration directing stdout and stderr into journal).

fschulze commented 5 years ago

@vlcinsky as far as I can tell, systemd by default redirects stdout and stderr to journalctl. I'm not sure if it's joining them into one stream though, which would be preferable for readability of the logs. I haven't used devpi with systemd yet. I'll most likely do it for m.devpi.net soon though.

vlcinsky commented 5 years ago

@fschulze I did the test of systemd unit yesterday. Findings:

devpi-server (as of version 4.9.0 in default configuration) emits following log entries:
- initial Serving on http://localhost:3141 to stdout
- all remaining log entries go to stderr
systemd service unit deals with logging:
- directive StandardError (man systemd.exec) defaults to value of DefaultStandardError in global systemd-system.conf, which is inherit.
  - inherit means "duplicate the file descriptor (here stderr) to stdout"
- directive StandardOutput (man systemd.exec) defaults to value of DefaultStandardOutput in global systemd-system.conf, which is journal
  - journal means "write to journal, which is accessible via journalctl".

The latest template for devpi.service looks as follows:

    [Unit]
    Description=Devpi Server
    Requires=network-online.target
    After=network-online.target

    [Service]
    Type=simple
    Restart=on-success
    ExecStart={devpibin} {server_args}
    User={user}
    SuccessExitStatus=SIGKILL

    [Install]
    WantedBy=multi-user.target

This result in both stdout and stderror log entries to be directed to journalctl.

In case, systemd defaults are different, it shall be fine as it will shall follow the (non-default) intentions of sysadmin.

I use my own devpi.service as described in snippet

    Description=Private pypi server by devpi-server
    Requires=network-online.target
    After=network-online.target

    [Service]
    Restart=always
    # ExecStart:
    # - shall point to existing devpi-server executable
    # - shall not use `--start`. We want the devpi-server to start in foreground
    ExecStart=/home/devpi/apps/devpi-server/.venv/bin/devpi-server --host 0.0.0.0
    # set User according to user which is able to run the devpi-server
    User=devpi

    [Install]
    WantedBy=multi-user.target

The (latest master version) devpi-server generated devpi.service has few directives, which can be modified:

Type can be omitted as it defaults to simple
SuccessExitStatus shall be removed as it probably applies only to forking service type. I do not use that and do not see any problems.
ExecStart shall make sure, the {server_args} do not include --start as it would break foreground execution model.

fschulze commented 5 years ago

@vlcinsky thanks, I will remove Type and SuccessExitStatus and add your ExecStart and User comments to the template

vlcinsky commented 5 years ago

Another small detail:

devpi-server logs by default to stderr
The Twelve factor app recommends to write the logs to stdout.

Consider modifying stream handler configuration at https://github.com/devpi/devpi/blob/afa9484f5a22aff20ea1084a34eda30282c34a76/server/devpi_server/log.py#L20

Using stream=sys.stdout within basicConfig would probably do the work.

fschulze commented 5 years ago

Thanks for the hint

fschulze commented 5 years ago

The process management options are deprecated and will be removed in devpi-server 6.0.0. The example templates have been expanded.

devpi / devpi

"devpi-server --status" does not work anymore due to waitpid() in isrunning() #434

Concept

Install devpi-server, stop it

Create `devpi.service` unit file

Enable and start the `devpi` service

Commands to start/stop/status/log entries for devpi-server

Summary

quick start

Installation on MS Windows

Installation on Linux with System V style of process manager

Summary

devpi / devpi

"devpi-server --status" does not work anymore due to waitpid() in isrunning() #434

Concept

Install devpi-server, stop it

Create devpi.service unit file

Enable and start the devpi service

Commands to start/stop/status/log entries for devpi-server

Summary

quick start

Installation on MS Windows

Installation on Linux with System V style of process manager

Summary

Create `devpi.service` unit file

Enable and start the `devpi` service