Bug 238424

Summary: net/kafka: fix rc script
Product: Ports & Packages Reporter: Dmitry Wagin <dmitry.wagin>
Component: Individual Port(s)Assignee: freebsd-ports-bugs (Nobody) <ports-bugs>
Status: Closed FIXED    
Severity: Affects Only Me CC: timp87
Priority: --- Keywords: feature, patch
Version: LatestFlags: dmitry.wagin: maintainer-feedback-
Hardware: Any   
OS: Any   
Attachments:
Description Flags
kafka.diff
none
kafka.diff none

Description Dmitry Wagin 2019-06-08 18:02:57 UTC
Created attachment 204909 [details]
kafka.diff

fixed pidfile/piddir manipulation
added support daemon supervise and restart functionality
added requirement for log directory
fixed title for the daemon process
Comment 1 Dmitry Wagin 2019-06-08 18:30:53 UTC
Created attachment 204910 [details]
kafka.diff
Comment 2 Pavel Timofeev 2019-06-09 18:28:45 UTC
(In reply to Dmitry Wagin from comment #0)
Hi! Thank you. I'm sorry I didn't fully understand the patch.
Could you please explain those 4 items in yur 'fixed' list more.
Comment 3 Dmitry Wagin 2019-06-09 18:57:24 UTC
(In reply to timp87 from comment #2)

1. fixed pidfile/piddir manipulation

before patch when kafka_pidfile="/var/run/kafka.pid", the following occurred:
chown kafka:kafka /var/run


2. added support daemon supervise and restart functionality

> man daemon
-r Supervise and restart the program after a one-second delay if it has been terminated.
-R restart_delay_seconds Supervise and restart the program after the specified delay if it has been terminated.
-P supervisor_pidfile Write the ID of the daemon process into the supervisor_pidfile

The -P option is useful combined with the -r option as supervisor_pidfile contains the ID of the supervisor not the child.  This is especially important if you use -r in an rc script as the -p option will give you the child's ID to signal when you attempt to stop the service, causing daemon to restart the child.


3. added requirement for log directory

required_dirs="${kafka_log_dir}"
as a replacement [ -d "$kafka_log_dir" ] || mkdir -p "$kafka_log_dir"


4. fixed title for the daemon process

I think it is a good idea to always set the process name (-t ${name}), not only when kafka_syslog_output_enable="YES"
Comment 4 Dmitry Wagin 2019-06-09 19:08:35 UTC
(In reply to Dmitry Wagin from comment #3)
and maybe good idea remove KAFKA_RUNDIR it's no longer needed

-: ${kafka_pidfile:=%%KAFKA_RUNDIR%%/kafka.pid
+: ${kafka_pidfile:=/var/run/kafka.pid
Comment 5 Pavel Timofeev 2019-07-04 07:24:24 UTC
(In reply to Dmitry Wagin from comment #3)
I'm very happy about your feedback!
I'm preparing port update to 2.3.0 right now and I'm incorporating your patch there.
I looked at your patch once again. This is great! I've understood everything.

One thing I would like to ask about:
how can we pass -r and -R to daemon(8) without any changes to your patch?
It seems like we have to add new variable to rc script for this. Am I missing something?
Comment 6 Dmitry Wagin 2019-07-04 21:08:30 UTC
(In reply to timp87 from comment #5)

${name}_flags Arguments to call ${command} with.

This is standard rc.subr functionality.
Comment 7 Pavel Timofeev 2019-07-05 09:01:28 UTC
(In reply to Dmitry Wagin from comment #6)
Ah, that's right.

This PR may be closed in behalf of https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=239000


Thanks a lot!
Comment 8 Dmitry Wagin 2019-07-05 09:12:05 UTC
(In reply to timp87 from comment #7)

Please remove KAFKA_RUNDIR it's no longer needed!
MFS (Memory File System) /var/run does not work with it.