Suresh,
Thanks for considering my suggestions. A few responses below.
--
Øystein
>>>>> "ST" == Suresh Thalamati <[EMAIL PROTECTED]> writes:
>> * SYSCS_ONLINE_BACKUP_DATABASE_AND_ENABLE_LOG_ARCHIVE_MODE()
>> - Same comment on ONLINE as above
>> - Could not boolean parameters be used now?
ST> I am not sure boolean paramets work with stored procedures now.
ST> Even if the support available, just to be consistent with existing
ST> backup procedure parameters types, I think using INT is better for the
ST> new procedures also.
I agree.
>> * backupDatabaseAndEnableLogArchiveMode()
>> - Most of the code here is the same as in backupDatabase().
>> (Another argument for pushing this code down to a lower layer.)
>> To avoid code duplication, whether to enable archiving could have
>> been a flag to backupDatabase.
ST> Added this function to support implicit commit/rollback, This function
ST> may not be needed any more. I will just call the low level backup
ST> calls directly.
Good.
>> * SYSCS_DISABLE_LOG_ARCHIVE_MODE()
>> - Is checkBackupTransactionIsIdle() strictly necessary here? This
>> seems like an operation where failures could be handle at
>> statement level.
>>
ST> No. Idle check was necessary for issuing implicit commit to be
ST> consistent with other backup calls.
Is this really a backup call?
>> java/engine/org/apache/derby/iapi/store/raw/xact/TransactionFactory.java
>> * stopBackupBlockingOperations()
>> - Name indicates that backup blocking operations are stopped, but
>> javadoc says that only new ones are blocked. I think the name
>> is misleading.
>> - Javadoc should be revisited for typos.
ST> I can change them too :
ST> blockBackupBlockingOperations()
ST> unblockBackupBlockingOoperation()
ST> Any suggestions ?
I guess you mean unblockBackupBlockingOperations(). Names are OK. It
requires a bit of syntactic and semantic analysis to be grasp the
meaning of 'blockBackupBlocking', but I do not have a better
suggestion.
>> java/engine/org/apache/derby/impl/store/raw/xact/Xact.java
>> * setBackupBlockingState()/setUnblockBackupState()
>> - Names should be symmetric. (e.g., blockBackup/unblockBackup)
>> - Why do you have to wait for commit to unblock? Would it not be
>> sufficient to have completed the unlogged operations before
>> backup is started?
ST> May be backup can be allowed once the unlogged operation is complete,
ST> I have not looked in-depth to allow immediately after the unlog
ST> operations is complete, one case that I thought likely to have
ST> problem is JAR file removal on post commit conflicting with backup
ST> jar copy operation.
ST> For now, I think enforcing the COMMIT restriction is ok.
Enforcing the commit restriction is the root of the discussions we
have had on idle transactions and automatic commit. If it is
necessary, things would be more straight-forward.
I would think whether the COMMIT restriction is necessary or not,
depends on whether one at restore will be able to roll back these
operations. I guess an index is ok to back up after it has been
created since it should be possible to roll back the creation by
dropping the index should the transaction never commit.
--
Øystein
