Hi,
On Wed, Mar 19, 2008 at 12:59:30PM +0900, HIDEO YAMAUCHI wrote:
> Hi,
>
> I send the patch which I revised.
Thanks. A few notes/questions:
1. The backup mode processing seems to be overly complicated.
The check if this attribute is set is invoked six times and then
different actions taken. Is that necessary? Furthermore, there is
only one check if the instance is in backup mode. It somehow
defies logic. For example:
"STARTED")
if is_clear_backupmode_set; then
output=`dbasql dbmount`
else
output=`dbasql dbmount dbopen`
fi
;;
I don't understand why the instance is treated in two different
ways here. Even if it has to be so, it shouldn't depend on the
attribute setting but on the actual state of affairs, i.e. is the
instance in the backup mode or not. In short, the processing
became extremely complex and very hard to follow. It has to be
simplified.
Shouldn't it be something like this:
if instance is in backup mode
end the backup mode
fi
continue with start/mount as usual
2. I guess that alter system checkpoint is not necessary before
shutdown immediate.
3. is_clear_backup_mode and is_backup_mode are somewhat
confusing. Should probably be renamed, something like:
is_clear_backupmode_set and is_instance_in_backup_mode. Still
verbose, but at least easier to understand.
4. This looks somewhat strange:
count="`dbasql db_backup_mode | tr -d COUNT`"
If I understood your intention correctly, it would be more clear:
count="`dbasql db_backup_mode | sed 's/COUNT//'`"
5. Things around backup mode are too verbose. I'd remove quite a
few ocf_log statements.
> Regards,
> Hideo Yamauchi.
Cheers,
Dejan
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
