Hi Swathi,
Many thanks for the review, it's much appreciated. I have included some
comments below.
Regards
Neil
Swathi.Devulapalli at Sun.COM wrote:
> Hi Neil,
>
> Please find my comments below:
>
> 1. ids/ids_register:
> Line #: 60
> Stop_signal=9 What is the stop_signal you are referring to here?
> Did you mean SIGSTOP?
>
This is SIGKILL as documented in "man -s 3head signal", i.e.
SIGKILL 9 Exit Killed
> 2. ids/functions:
> a) I would suggest the location of LOGFILE be set to something like
> /var/cluster/logs/ha-ids/${RESOURCE}_logfile instead of /var/tmp/
> 27 LOGFILE=/var/tmp/${RESOURCE}_logfile
>
Agreed, I'll amend the functions file.
> b) You may want to do this check
> 197 if [ -f "${INFORMIXDIR}/etc/${ONCONFIG}" ]
> inside this condition
> if [ -d "${INFORMIXDIR}" ]
> 166 then
> 167 debug_message "Validate - ${INFORMIXDIR} exists"
>
I think we're okay, as we check if ${INFORMIXDIR} exists earlier on, i.e.
165 if [ -d "${INFORMIXDIR}" ]
166 then
167 debug_message "Validate - ${INFORMIXDIR} exists"
Unless I misread your comment.
> c) We could use a similar message in validate as well for the user
> to know that validate was successful. I see only debug/error
> messages
> in the validate function.
> # SCMSGS
> 325 # @explanation
> 326 # The specified Informix Server was started
> successfully.
> 327 # @user_action
> 328 # None required. Informational message.
> 329 scds_syslog -p daemon.notice -t $(syslog_tag) -m \
> 330 "start_ids - Informix Server (%s) started
> rc(%s)" \
> 331 "${INFORMIXSERVER}" "${rc}"
>
Agreed, I'll add a notice message to indicate a successful validate.
> 3. If Informix DS is already running outside sun cluster control, how
> is it handled?
> You bail-out or do something else?
The agent will tolerate if IDS was manually started, i.e. running
outside of cluster control. Within start_ids() I have the following,
296 # Turn off PMF restart if ${INFORMIXSERVER} has been
manually started
297 /usr/bin/sleep 120 &
298 /usr/cluster/bin/pmfadm -s
${RESOURCEGROUP},${RESOURCE},0.svc
299
300 # SCMSGS
301 # @explanation
302 # The specified Informix Server has been manually started.
303 # @user_action
304 # None required. Informational message.
305 scds_syslog -p daemon.notice -t $(syslog_tag) -m \
306 "start_ids - Informix Server (%s) was manually started" \
307 "${INFORMIXSERVER}"
Essentially, we turn off PMF monitoring and provide a token sleep pid
for the existing pmftag. Once that sleep expires, the pmftag is empty
but not monitored. The probe and stop can work unaffected by this event.
>
> 4. Is there any cleanipc kind of utility in Informix installation? If
> yes, you could use
> the same utility instead of cleanup_ipcs function. If not, this is
> fine too.
>
Not that I am aware of.
> 5. ids/ids_smf_register:
> I have executed this script just to check if xml file is created.
> At the outset, it failed with the
> error: syntax error at line 36 : `<<' unmatched
> Then I found out that the second EOF should have a literal TAB or
> NO SPACE before.
> 121 EOF
> Since I copied it from webrev, there were spaces in the .ksh file.
> I am sure
> you would have had a tab out there.
>
Yes, the EOF was tabbed out.
> 6. Except "Makefile" none of the files has a Sun Copyright. Is it not
> required?
Well, my understanding is that as this is on opensolaris project we just
need the CDDL Header START/END statement. As such the Sun Copyright
should be removed from the Makefile.
>
> 7. ids/ids_config:
> I am not sure how much this information
> " To use the Sun Cluster Data Service for Informix Dynamic Server
> 58 # together with the Sun Cluster Data Service for Solaris
> Containers"
>
> is required here. But this should definitely be captured in the doc
> guide.
I agree, it's probably too much information as most of the time I expect
that the Container agent wil not be deployed, i.e. failover zones won't
be used.
>
> Thanks,
Thank-you Swathi.
> Swathi
>
>
>> Neil Garthwaite wrote:
>>
>>
>>> Hi Everyone,
>>>
>>> The HA-Informix project page
>>> http://opensolaris.org/os/project/ha-informix has been updated to
>>> include the initial code for the HA-Informix agent.
>>>
>>> I'd welcome a code review for this agent, a webrev can be found here
>>>
>>> http://cr.opensolaris.org/~neilg/ohacds-informix
>>>
>>> Alternatively, the agent is available for download. Please use the
>>> accompanying cheatsheet if you want to try out the agent.
>>>
>>> http://opensolaris.org/os/project/ha-informix/cheatsheet_ohac_ids.pdf
>>>
>>> Finally, many thanks to everyone that reviewed the Design Doc and
>>> Caller Sequence Doc, I've updated those docs to reflect your comments.
>>>
>>> Regards
>>> Neil
>>>
>>
>> _______________________________________________
>> ha-clusters-discuss mailing list
>> ha-clusters-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/ha-clusters-discuss
>>
>>
>>
