Victor Kirkebo wrote:
> Roland Mainz wrote:
> > Victor Kirkebo wrote:
> >>Kindly find the webrev for memcached integration at :
> >>http://cr.opensolaris.org/~vk136562/memcached/
> >>
> >>Code reviews are solicited from this team and review/feedback is
> >>requested by 11/20.
> >
> >A quick 5min race over
> >http://cr.opensolaris.org/~vk136562/memcached/memcached.patch (patch
> >code is quoted as "> "):
> >
> >>--- /dev/null Mon Nov 19 03:47:27 2007
> >>+++ new/usr/src/lib/memcached/Makefile.sfw Mon Nov 19 03:47:27 2007
> >>
> >[snip]
> >
> >>+# Memcached 1.2.2
> >>+# Libmemcache 1.4.0
> >>+
> >>+VERMEMCACHED=memcached-1.2.2
> >>+VERLIBMEMCACHE=libmemcache-1.4.0.rc2
> >>+
> >>+include ../Makefile.lib
> >>+
> >>+CFLAGS += -mt -xO5 -xipo -xtarget=generic
> >
> >1. Please add "-xstrconst"
>
> Done
Thanks! :-)
> >2. AFAIK you want "-xipo=2".
>
> Unfortunately we do not have enough time to do the necessary testing to
> verify this option for this release.
> However, we would like to try this out in for future releases.
Ok...
... BTW: Did you check the logs that "xipo" is used at _link_ time, too
(a common problem with the GNU stuff is that they sometimes don't pass
CFLAGS to the link stage (which would render "xipo" useless since the
all-in-one optimisation is done when "cc" is called for the final link
pass)) ?
> >3. Are you sure you want -xO5 ? This optimisation level enables lots of
> >magic and should only be used with care, e.g. does "memcached" have a
> >testsuite or something like that which can gurantee that the code still
> >works ?
>
> Again, we have used this in our testing and would like to keep it for now.
Ok...
BTW: Check whether -xspace makes any difference - sometimes smaller code
is better...
[snip]
> >AFAIK you can turn this into one "rm" call, e.g.
> >-- snip --
> > -rm -rf \
> > $(VERMEMCACHED) \
> > $(VERLIBMEMCACHE)
> >-- snip --
>
> Done
Thanks! :-)
> >>--- /dev/null Mon Nov 19 03:47:27 2007
> >>+++ new/usr/src/lib/memcached/install-sfw Mon Nov 19 03:47:27 2007
> >>@@ -0,0 +1,68 @@
> >>+#!/bin/sh
> >[snip]
> >
> >>+# Memcached 1.2.2 is dependent on libevent
> >>+# Libmemcached 1.4.0 is a C API for memcached
> >
> >Standard template for this kind of script:
> >-- snip --
> >AFAIK it may be nice to use /usr/bin/ksh or /usr/bin/ksh93 (or "bash" if
> >it supports the "errexit" flag) for this script and then do a $ set -o
> >errexit # at the beginning that the script - the "errexit" flag causes
> >the shell to immediately abort the script when it hits a command which
> >returns a failure instead of continuing operation (and maybe running
> >amok (and ordering 500 new ${product.expensive} via your ${bankaccount}
> >etc. =:-) ))
> >-- snip --
>
> We might consider ksh/ksh93 for future releases.
Ok...
... but there is a giant catch with the SMF script below (see issue
'SIGHUP vs. "disown"' below...) ...
[snip]
> >Quotes around variable expansions would be nice, e.g. ${x}/foo ---->
> >"${x}/foo" - this should limit the damage generated by errors, e.g. when
> >a variable contains garbage (e.g. usage message in stdout)
> >
> >
> Done
Thanks! :-)
> >>+MANSCRIPT=sunman-stability
> >>+. ${SRC}/tools/install.subr
> >
> >If you use ksh93 or bash as suggested above replace the "." with
> >"source" ...
> >
> >>+_install N memcached.xml ${MANIFESTDIR}/memcached.xml 444
> >>+_install N memcached ${METHODDIR}/memcached 555
> >>+_install N libmemcache.html ${DOCMEMCACHEDDIR}/libmemcache.html 444
> >>+_install M memcached.1m ${MAN1MDIR}/memcached.1m 444
> >>+_install M libmemcache.3lib ${MAN3LIBDIR}/libmemcache.3lib 444
> >>+
> >>+cd ${VERMEMCACHED}
> >>+
> >>+_install E ./memcached ${LIBDIR}/memcached 555
> >>+
> >>+cd ../${VERLIBMEMCACHE}
> >>+
> >>+_install N ./src/.libs/libmemcache.a ${LIBDIR}/libmemcache.a 444
> >>+_install D ./src/.libs/libmemcache.so.0.4.0 ${LIBDIR}/libmemcache.so.0.4.0
> >>555
> >>+_install N ./include/memcache.h ${INCMEMCACHEDDIR}/memcache.h 444
> >>+_install L libmemcache.so.0.4.0 ${LIBDIR}/libmemcache.so
> >>+_install L libmemcache.so.0.4.0 ${LIBDIR}/libmemcache.so.0
> >>+
> >>+exit 0
> I've added quotes here.
Ok...
[snip]
> >>case "$1" in
> >>'start')
> >>$MCBIN/memcached $MCOPTIONS &
> >>
> >>
> >
> >Erm... AFAIK if I recall it correctly this will not work. If the
> >terminal goes away the "memcached" will get SIGHUP (and likely die).
> >AFAIK the only fixes are: a) Add some glue to "memcached" to ignore
> >SIGHUP or b) switch to ksh93 which has the "disown" builtin command
> >which will prevent the SIGHUP...
>
> I used your suggestion from the attached file:
>
> case "$1" in
> 'start')
> "${MCBIN}/memcached" $MCOPTIONS &
> # detach deamon from shell to avoid that it gets a SIGHUP
> ;;
Erm... where is the "disown" call ? The point was to call "disown" top
make sure the background process is completely detached and won't be
hit&&run-over&&squished by the "SIGHUP" bus.
If you can't use ksh93's "disown"-builtin please add a small line of
code to "memcached" which causes SIGHUP to be ignored in the deamon
itself.
[snip]
> >Uhm... the $"mymsg" string literals only work in bash or ksh93 (and I'm
> >not sure whether the SMF scripts should be localised or not... (IMO you
> >can keep it (and a l10n catalog file for the "C" locale would be nice
> >that the l10n people at Sun get a template...) but please sync with
> >smf-discuss at opensolaris.org)).
>
> Removed $
Thanks! :-)
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) roland.mainz at nrubsig.org
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 7950090
(;O/ \/ \O;)
