Bug 161109 - Fix rc.d script for databases/memcached
Summary: Fix rc.d script for databases/memcached
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Steve Wills
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-28 22:40 UTC by Doug Barton
Modified: 2011-10-05 21:09 UTC (History)
0 users

See Also:


Attachments
file.diff (3.96 KB, patch)
2011-09-28 22:40 UTC, Doug Barton
no flags Details | Diff
smime.p7s (4.80 KB, application/pkcs7-signature)
2011-10-03 21:15 UTC, vedad
no flags Details
memcached_rc_fix.diff (747 bytes, patch)
2011-10-04 01:53 UTC, Steve Wills
no flags Details | Diff
memcached_rc_fix.diff.sig (287 bytes, application/octet-stream)
2011-10-04 01:53 UTC, Steve Wills
no flags Details
memcached-rcd.diff (1.03 KB, patch)
2011-10-04 06:00 UTC, Doug Barton
no flags Details | Diff
smime.p7s (4.80 KB, application/pkcs7-signature)
2011-10-05 19:53 UTC, vedad
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Doug Barton freebsd_committer 2011-09-28 22:40:05 UTC
	The rc.d script for memcached has various problems:

	1. Creating /var/run/memcached in the Makefile does nothing for
	   package installs.
	2. Because the service runs as an unprivileged user it must
	   REQUIRE: LOGIN
	3. The rc.d script should create the directory in /var/run, and give
	   it appropriate permissions.
	4. The rc.d script should clean up pidfiles since the software doesn't
	5. Various other shell cleanups.

Fix: Apply the following patch:
How-To-Repeat: 	DNA
Comment 1 Edwin Groothuis freebsd_committer 2011-09-28 22:40:16 UTC
Responsible Changed
From-To: freebsd-ports-bugs->swills

Over to maintainer (via the GNATS Auto Assign Tool)
Comment 2 dfilter service freebsd_committer 2011-09-30 14:52:39 UTC
swills      2011-09-30 13:52:30 UTC

  FreeBSD ports repository

  Modified files:
    databases/memcached  Makefile 
    databases/memcached/files memcached.in 
  Log:
  - Fix rc.d script for databases/memcached
  
  PR:             ports/161109
  Submitted by:   dougb
  
  Revision  Changes    Path
  1.53      +2 -3      ports/databases/memcached/Makefile
  1.7       +27 -17    ports/databases/memcached/files/memcached.in
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 3 Steve Wills freebsd_committer 2011-09-30 14:52:43 UTC
State Changed
From-To: open->closed

Committed. Thanks!
Comment 4 vedad 2011-10-03 21:15:59 UTC
Hi,

This fix introduces additional problems.

When running multiple profiles, running "memcached stop <profile>" 
deletes all pid files for all profiles.

In a similar way, "memcached stop" will stop the first profile only, but 
will delete all pid files.

Therefore, consequent "memcached stop <profile>" will fail. Worse, 
"memcached restart" will launch additional daemons without stopping 
previous ones, possibly leading to memory exhaustion.


Regards,

-- 
Vedad KAJTAZ
Conseil en systèmes informatiques

vedad@kajtaz.net
http://vedad.kajtaz.net/
8 Avenue des Marronniers
94120 Fontenay-sous-bois, FRANCE
GSM: +33 6 74 89 32 12
Tel: +33 1 83 62 47 16 / Fax: +33 1 83 62 47 42
Comment 5 Steve Wills freebsd_committer 2011-10-04 01:53:28 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

On 10/03/11 16:40, Vedad KAJTAZ wrote:
>  Hi,
>  
>  This fix introduces additional problems.
>  
>  When running multiple profiles, running "memcached stop <profile>"=20
>  deletes all pid files for all profiles.
>  
>  In a similar way, "memcached stop" will stop the first profile only, but =
>  
>  will delete all pid files.
>  
>  Therefore, consequent "memcached stop <profile>" will fail. Worse,=20
>  "memcached restart" will launch additional daemons without stopping=20
>  previous ones, possibly leading to memory exhaustion.
>  

Thanks for letting me know. Could you try the attached patch? I've
tested a bit and I was able to reproduce the issue you reported, except
for the duplicate memcached processes and I believe this patch fixes it.

Thanks,
Steve
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (FreeBSD)

iQEcBAEBAgAGBQJOilkGAAoJEPXPYrMgexuh9tUH/iQMWxvQOvr3wY749u8beXZo
tDqDpYb2Yu3WL7iux8Il5mD17PIkmDCPkScEUuiKVdJ1WEeuex73rFlgjz7sbWy9
rrfVGBFIkpj+bjLb7N9X9HIO8ADK/hdR/kKUujv879N4Hd0D5+td/w7HE2OMneQk
+5Pg+dIIM2vECAdjzwKEhNtVwTtH2U9HVcbjMtOfaryrKKtEzWvr/b+24WgUBzyT
Zv/QTwNv0rKWCb8rb138eE4//8BGBQ4l03WsDtFCrvNyMZOjdmUzZxXko1DbB3WS
MEnxiPOzUtyA8ZtKU/Xo4CJzfonVa1IMjiGATvCYidY8zzdrjX6PaN7ATpimQbw=
=D+OJ
-----END PGP SIGNATURE-----
Comment 6 Doug Barton freebsd_committer 2011-10-04 06:00:45 UTC
On 10/03/2011 17:53, Steve Wills wrote:

> Thanks for letting me know.

Me too, and sorry for the hassle.

> Could you try the attached patch? I've
> tested a bit and I was able to reproduce the issue you reported, except
> for the duplicate memcached processes and I believe this patch fixes it.

The attached is slightly cleaner. If a profile is specified then
$profile will be set, and $pidfile will be set appropriately. Also, to
use unlink you have to first test that the file exists. Otherwise if it
doesn't, you'll get an error. It's still safer to use unlink than 'rm
-f' though, even if only a little bit.

If no profile is specified on the command line we want to delete all the
pidfiles unconditionally.


hth,

Doug

-- 

	Nothin' ever doesn't change, but nothin' changes much.
			-- OK Go

	Breadth of IT experience, and depth of knowledge in the DNS.
	Yours for the right price.  :)  http://SupersetSolutions.com/
Comment 7 dfilter service freebsd_committer 2011-10-04 14:43:47 UTC
swills      2011-10-04 13:43:37 UTC

  FreeBSD ports repository

  Modified files:
    databases/memcached  Makefile 
    databases/memcached/files memcached.in 
  Added files:
    databases/memcached/files patch-sasl_defs.c 
  Log:
  - Fix issue with RC script [1]
  - Fix build with SASL support enabled [2]
  
  PR:             ports/161109
  Reported by:    Vedad KAJTAZ <vedad@kajtaz.net> [1]
  Reported by:    Ben Tung <benpptung@tacol.biz> [2]
  Submitted by:   dougb [1]
  
  Revision  Changes    Path
  1.54      +1 -1      ports/databases/memcached/Makefile
  1.8       +14 -7     ports/databases/memcached/files/memcached.in
  1.1       +19 -0     ports/databases/memcached/files/patch-sasl_defs.c (new)
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 8 swills 2011-10-04 14:46:24 UTC
Doug's patch looked good, so I went ahead and committed it along with
another change I had queue'd. If either of you notice any problems, please
let me know.

Thanks,
Steve

> Hi,
>
> Thanks for such a quick reply!
>
> I will test both patches this evening, and let you know whether they
> both fix the problem (I will cc bug-followups).
>
> Regards,
>
> Le 04/10/2011 07:00, Doug Barton a écrit :
>> On 10/03/2011 17:53, Steve Wills wrote:
>>
>>> Thanks for letting me know.
>>
>> Me too, and sorry for the hassle.
>>
>>> Could you try the attached patch? I've
>>> tested a bit and I was able to reproduce the issue you reported, except
>>> for the duplicate memcached processes and I believe this patch fixes
>>> it.
>>
>> The attached is slightly cleaner. If a profile is specified then
>> $profile will be set, and $pidfile will be set appropriately. Also, to
>> use unlink you have to first test that the file exists. Otherwise if it
>> doesn't, you'll get an error. It's still safer to use unlink than 'rm
>> -f' though, even if only a little bit.
>>
>> If no profile is specified on the command line we want to delete all the
>> pidfiles unconditionally.
>>
>>
>> hth,
>>
>> Doug
>>
>
>
> --
> Vedad KAJTAZ
> Conseil en systèmes informatiques
>
> vedad@kajtaz.net
> http://vedad.kajtaz.net/
> 8 Avenue des Marronniers
> 94120 Fontenay-sous-bois, FRANCE
> GSM: +33 6 74 89 32 12
> Tel: +33 1 83 62 47 16 / Fax: +33 1 83 62 47 42
>
>
Comment 9 vedad 2011-10-05 19:53:44 UTC
Hi,

Thanks, the patch has indeed fixed the problem.

May I suggest another "cosmetic" fix:

When providing a misspelled command with profiles enabled, the error 
message is printed as many times as there are profiles.

e.g:

$ /usr/local/etc/rc.d/memcached restat
===> memcached profile: dev
/usr/local/etc/rc.d/memcached: unknown directive 'restat'.
Usage: /usr/local/etc/rc.d/memcached 
[fast|force|one](start|stop|restart|rcvar|status|poll)
===> memcached profile: preprod
/usr/local/etc/rc.d/memcached: unknown directive 'restat'.
Usage: /usr/local/etc/rc.d/memcached 
[fast|force|one](start|stop|restart|rcvar|status|poll)
===> memcached profile: prod
/usr/local/etc/rc.d/memcached: unknown directive 'restat'.
Usage: /usr/local/etc/rc.d/memcached 
[fast|force|one](start|stop|restart|rcvar|status|poll)


Regards,


Le 04/10/2011 15:46, Steve Wills a écrit :
> Doug's patch looked good, so I went ahead and committed it along with
> another change I had queue'd. If either of you notice any problems, please
> let me know.
>
> Thanks,
> Steve
>
>> Hi,
>>
>> Thanks for such a quick reply!
>>
>> I will test both patches this evening, and let you know whether they
>> both fix the problem (I will cc bug-followups).
>>
>> Regards,
>>
>> Le 04/10/2011 07:00, Doug Barton a écrit :
>>> On 10/03/2011 17:53, Steve Wills wrote:
>>>
>>>> Thanks for letting me know.
>>>
>>> Me too, and sorry for the hassle.
>>>
>>>> Could you try the attached patch? I've
>>>> tested a bit and I was able to reproduce the issue you reported, except
>>>> for the duplicate memcached processes and I believe this patch fixes
>>>> it.
>>>
>>> The attached is slightly cleaner. If a profile is specified then
>>> $profile will be set, and $pidfile will be set appropriately. Also, to
>>> use unlink you have to first test that the file exists. Otherwise if it
>>> doesn't, you'll get an error. It's still safer to use unlink than 'rm
>>> -f' though, even if only a little bit.
>>>
>>> If no profile is specified on the command line we want to delete all the
>>> pidfiles unconditionally.
>>>
>>>
>>> hth,
>>>
>>> Doug
>>>
>>
>>
>> --
>> Vedad KAJTAZ
>> Conseil en systèmes informatiques
>>
>> vedad@kajtaz.net
>> http://vedad.kajtaz.net/
>> 8 Avenue des Marronniers
>> 94120 Fontenay-sous-bois, FRANCE
>> GSM: +33 6 74 89 32 12
>> Tel: +33 1 83 62 47 16 / Fax: +33 1 83 62 47 42
>>
>>
>
>
>



-- 
Vedad KAJTAZ
Conseil en systèmes informatiques

vedad@kajtaz.net
http://vedad.kajtaz.net/
8 Avenue des Marronniers
94120 Fontenay-sous-bois, FRANCE
GSM: +33 6 74 89 32 12
Tel: +33 1 83 62 47 16 / Fax: +33 1 83 62 47 42