Re: dmd: Unload one or all services at runtime.

[ludo@gnu.org (Ludovic Courtès)]

Hi!

Alex Sassmannshausen <alex.sassmannshausen@gmail.com> skribis:

> The attached patch is a first step towards this aim. It allows you to
> unload individual services (by their name) or all known user
> services. It even allows you to unload the special service dmd itself,
> which is the same as sending the stop command to dmd.
>
> For example:
> $: dmd rm dmd apache // Unload the apache server
> $: dmd rm dmd web-server // Unload the service providing
>                          // a web server if there is only one.

What happens here if there’s a service running that depends on
‘web-server’?  I’m guessing it should be stopped.

> $: dmd rm dmd all // Unload all user services.
>
> You can then reload the relevant service's definition (or, if you ran
> 'dmd rm dmd all', you can reload your dmd.d/init.scm).
>
> In future this might provide the foundation for a 'reload' action for
> dmd.

Cool!

> From 1b4ec0f2261e1231ff21c5486dc6e75466c5829e Mon Sep 17 00:00:00 2001
> From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
> Date: Sun, 23 Feb 2014 11:06:14 +0100
> Subject: [PATCH] dmd: Add dmd action rm: remove known services.
>
> * modules/dmd/service.scm (deregister-services): New procedure.
>   (dmd-service): Add new action: rm.
> * dmd.texi (The 'dmd' and 'unknown' services): Document 'rm'.

There are 3 terms used here: remove, unload, and deregister.  For
consistency, I would stick to just one, perhaps ‘unload’.  WDYT?

> +@item rm @var{service-name}
> +Attempt to remove the service identified by @var{service-name}.
> +@command{dmd} will first stop the service, if necessary, and then
> +remove it from the list of registered services.  If @var{service-name}
> +simply does not exist, output a warning and do nothing.  If it exists,
> +but is provided by several services, output a warning and do nothing.

Mention what happens to services depending on it.

> +This latter case might occur for instance with the fictional service
> +web-server, which might be provided by both apache and nginx.  If

Should be @code{web-server}, @code{apache}, and @code{nginx}.

> +(define (deregister-service service-name)
> +  "For each string in SERVICE-NAME, stop the associated service if
> +necessary and remove it from the services table.  If SERVICE-NAME is
> +the special string 'all', remove all services except for dmd.
> +
> +This will remove a service either if it is identified by its canonical
> +name, or if it is the only service providing the service that is
> +requested to be removed."
> +  (define (deregister service)
> +    (if (running? service)
> +        (stop service))
> +    ;; Remove services provided by service from the hash table.
> +    (for-each
> +     (lambda (name)
> +       (let ((old (lookup-services name)))
> +         (if (= 1 (length old))
> +             ;; Only service provides this service, ergo:
> +             (begin
> +               ;; Reduce provided services count
> +               (set! services-cnt (1- services-cnt))
> +               ;; Remove service entry from services.
> +               (hashq-remove! services name))
> +             ;; ELSE: remove service from providing services.
> +             (hashq-set! services name
> +                         (remove
> +                          (lambda (lk-service)
> +                            (eq? (canonical-name service)
> +                                 (canonical-name lk-service)))
> +                          old)))))
> +     (provided-by service)))
> +  (define (service-pairs)
> +    "Return '(name . service) of all user-registered services."
> +    (filter (lambda (service-pair) (if service-pair #t #f))
> +            (hash-map->list
> +             (lambda (key value)
> +               (let ((can-name (canonical-name (car value))))
> +                 (if (and (null? (cdr value))
> +                          (eq? key can-name)
> +                          (not (eq? can-name 'dmd)))
> +                     (cons key (car value)) #f)))

Note that the two arms of ‘if’ should always be aligned.
Also, (ice-9 match) can help here IMO:

  (lambda (key value)
    (match value
      ((service)    ; only one service associated with KEY
       (and (eq? key (canonical-name service))
            (not (eq? key 'dmd))
            (cons key service)))
      (_ #f)))      ; two or more services associated with KEY

> +           ;; Removing only one service.
> +           (let ((services (lookup-services name)))
> +             (cond ((null? services)
> +                    (local-output "'~a' is an uknown service." name))
> +                   ((= 1 (length services))
> +                    ;; Are we removing a user service…
> +                    (if (eq? (canonical-name (car services)) name)
> +                        (local-output "Removing service '~a'..."
> +                                      name)
> +                        ;; or a virtual service?
> +                        (local-output
> +                         (string-append "Removing service '~a' "
> +                                        "providing '~a'...")
> +                         (canonical-name (car services)) name))
> +                    (deregister (car services))
> +                    (local-output "Done."))
> +                   (else
> +                    ;; Service name to ambiguous
> +                    (local-output
> +                     (string-append "'~a' identifies more than one "
> +                                    "service to be stopped: '~a'.")
> +                     name (map canonical-name services)))))))))

Likewise:

  (match (lookup-services name)
    (()             ; unknown service
     ...)
    ((service)      ; only SERVICE provides NAME
     ...)
    ((services ...) ; ambiguous NAME
     ...))

What about adding a test case, as a shell script (since that’s what we
have currently)?  I’m thinking of something running dmd with a
configuration file that contains a service definition, then running
‘deco unload foo’, and then making sure that ‘deco status dmd’ doesn’t
list it any longer.

Bonus point if there’s another service depending on the one we’re
unloading.

WDYT?  Could you send an updated patch?

Thanks!

Ludo’.