You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Øystein Grøvlen (JIRA)" <de...@db.apache.org> on 2006/01/02 17:35:02 UTC
[jira] Commented: (DERBY-239) Need a online backup feature that does not block update operations when online backup is in progress.
[ http://issues.apache.org/jira/browse/DERBY-239?page=comments#action_12361535 ]
Øystein Grøvlen commented on DERBY-239:
---------------------------------------
I have reviewed the latest patches (3 through6). (A bit late I must admit). Here are my comments:
java/engine/org/apache/derby/catalog/SystemProcedures.java
* SYSCS_BACKUP_DATABASE()
- "By default this procedure will wait ..." Is it possible to
change this behavior for this particular procedure? If not,
"by default" is a bit misleading
- "wait for the backup blocking unlogged operations to complete
..." is a bit heavy. I suggest just "wait for any unlogged
operations to complete ..."
* SYSCS_ONLINE_BACKUP_DATABASE()
- Since both backup procedures are ONLINE, it is a bit misleading
to use this word to distinguish between the two backup
procedures. I guess the main reason for choosing a new name is
the extra parameter. In that case, I think it would be better
to name the new procedure, SYSCS_BACKUP_DATABASE_NOWAIT and
leave out the parameter.
- The javadoc does not say what will happen if wait is 0. Will one
get an exception if there is unlogged operations?
* backupDatabase()
- Is this the right layer for checking that the transaction is
idle and for doing rollback/commit the transaction? Since this
is a requirement for the logic at lower layers to work
correctly, not something that is done because it is the
desirable behavior of the system procedure, I feel that this
should be done at a lower layer.
- I know when we discussed this isssue earlier, I agreed that
checking that the transaction is idle was a good solution.
However, thinking a bit more about this, I think it would be
better to fail the transaction when unlogged operations have
been performed by the same transaction. That would limit it to
those who actually need to be affected, and it would
significantly reduce the probability of someone ever
experiencing this problem.
- I am not very fond of automatic commits like this. If this is
necessary, I think it would be better to require that backup is
performed in autocommit mode. Then the implications would be
more evident to the users and not catch someone by surprise.
- The javadoc for the system procedures that use this function
should state the requirement imposed here (idle transaction) and
that the transaction will be committed if backup is succesful.
* SYSCS_ONLINE_BACKUP_DATABASE_AND_ENABLE_LOG_ARCHIVE_MODE()
- Same comment on ONLINE as above
- Could not boolean parameters be used now?
* 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.
* SYSCS_DISABLE_LOG_ARCHIVE_MODE()
- Is checkBackupTransactionIsIdle() strictly necessary here? This
seems like an operation where failures could be handle at
statement level.
java/engine/org/apache/derby/iapi/store/access/AccessFactory.java
* backup(String ...)
- "Please see cloudscape on line ..." Derby?
* backup(File ...)
- I will just remind you of DERBY-665. I could do that work, but
I am not sure it is a good idea for other people change the
backup code while you are working on it. (May create merge
conflicts for you.)
java/engine/org/apache/derby/iapi/store/raw/xact/RawTransaction.java
* setBackupBlockingState()
- I do not like the name for this method. I suggest calling it
blockBackup() or something like that. At least, the javadoc
should explain what is meant by "backup blocking state".
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.
java/engine/org/apache/derby/impl/db/BasicDatabase.java
* backupAndEnableLogArchiveMode()
- Non-standard indentation for parameters?
java/engine/org/apache/derby/impl/store/raw/RawStore.java
* backup(String, boolean)
- Typo in comment: "Check if there any backup ..." Remove "there"?
* backupAndEnableLogArchiveMode(String, boolean)
- Why do you need a finally clause? Would not a catch clause be
sufficient? Then, you could eliminate the local 'error'
variable.
java/engine/org/apache/derby/impl/store/raw/data/RFResource.java
* add()/remove()
- Are the casts to RawTransaction safe? Does this assumption have
any impact on the modularity of the code?
* serviceImmediately()
- How is this change related to backup?
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?
java/engine/org/apache/derby/impl/store/raw/xact/XactFactory.java
* canStartBackupBlockingOperation()
- Since this method now may wait for backup to complete, I do not
feel canStartBackupBlockingOperation() is a good name.
- Symmetric naming with backupBlockingOperationFinished() would
make it more evident that these two functions should be called
in pairs.
java/engine/org/apache/derby/loc/messages_en.properties
* XSRSA.S
- Suggest the following change:
"Cannot backup the database when unlogged operations are uncommitted. Please commit the transactions with backup blocking operations or use the backup procedure with option to wait for them to complete."
- Generally, I am not very fond of these long error messages. I
think it would be better with just a single sentence, and then
the user should be able to look up an explanation in a manual.
* XSRSB.S
- Suggest the following change:
"Backup operation can not be performed in an active transaction. Please start a new transaction to execute backup procedures."
java/testing/org/apache/derbyTesting/functionTests/tests/store/OnlineBackupTest1.java
* runTest()
- Should have more than one unlogged operation in parallel in
order to fully test that the counting of unlogged operations
work.
- Suggest to add test that when doing backup in a non-idle
transaction, previous work has not been rolled back when backup
fails, and that one can continue with more operations within the
same transaction.
* runConsistencyChecker()
- This does only check consistnecy of internal structures. Should
also check consistency of application data. Maybe you could
execute a select?
* performDmlActions()
- I assume the intention here is to do "while (!stopActivity)"
* endUnloggedAction()
- What is the role of the insert? Is is unlogged? It is not
evident from the method name or Javadoc why you are doing
inserts here.
* select()
- What is the point of doing consistency checks on columns that
are not updated? If id and name does not match, that must be
caused by errors in code that is not particular to backup.
java/testing/org/apache/derbyTesting/functionTests/tests/store/OnlineBackupTest3.java
* installJarTest()
- Typos in comment: "// followng backup call should because jar
operation is pending". Should what?
- Comment say: "//Now commit the jar operation in connection1 for
backup to proceed." The next statement does an insert. This is
confusing.
- It would be nice if the test checked that jar operation is still
waiting for backup when create index has completed, but I guess
this is a bit difficult to achieve.
* A mix of tab and spaces for indentation. For new files that
should not be necessary!
* removeJarTest()
- Comment copied from installJarTest?
"// wait for customer app jar installation to finish now. "
java/testing/org/apache/derbyTesting/functionTests/tests/store/onlineBackupTest2.sql
* Do you have idea of how frequently it happens that backup thread
has not been blocked yet when backupdir is created? How long a
sleep would you need to be certain?
> Need a online backup feature that does not block update operations when online backup is in progress.
> --------------------------------------------------------------------------------------------------------
>
> Key: DERBY-239
> URL: http://issues.apache.org/jira/browse/DERBY-239
> Project: Derby
> Type: New Feature
> Components: Store
> Versions: 10.1.1.0
> Reporter: Suresh Thalamati
> Assignee: Suresh Thalamati
> Attachments: obtest_customer.jar, onlinebackup.html, onlinebackup1.html, onlinebackup_1.diff, onlinebackup_2.diff, onlinebackup_3.diff, onlinebackup_4.diff, onlinebackup_5.diff, onlinebackup_6.diff
>
> Currently Derby allows users to perfoms online backups using SYSCS_UTIL.SYSCS_BACKUP_DATABASE() procedure, but while the backup is in progress, update operations are temporarily blocked, but read operations can still proceed.
> Blocking update operations can be real issue specifically in client server environments, because user requests will be blocked for a long time if a
> backup is in the progress on the server.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
Re: [jira] Commented: (DERBY-239) Need a online backup feature that
does not block update operations when online backup is in progress.
Posted by Øystein Grøvlen <Oy...@Sun.COM>.
>>>>> "MM" == Mike Matrigali <mi...@sbcglobal.net> writes:
MM> I agree, at this point I think what you have implemented is a useful
MM> feature. It is much more online than 10.1 version. I agree with
MM> Oystein there may be ways to improve - but maybe those can be logged
MM> as new improvements.
I agree.
--
Øystein
Re: [jira] Commented: (DERBY-239) Need a online backup feature that
does not block update operations when online backup is in progress.
Posted by Mike Matrigali <mi...@sbcglobal.net>.
I agree, at this point I think what you have implemented is a useful
feature. It is much more online than 10.1 version. I agree with
Oystein there may be ways to improve - but maybe those can be logged
as new improvements.
Suresh Thalamati wrote:
> Øystein Grøvlen wrote:
>
>> Suresh,
>>
>> Thanks for considering my suggestions. A few responses below.
>>
>> --
>> Øystein
>>
>
>> >> * 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?
>>
>
> Not a real backup call, but it related to the backup functionality and
> disabling the log archive mode operation can not be rolled back, that
> was the reason behind doing the implicit commit for this call also. I
> will be removing the IDLE check and implicit commit in this case also
> along with other backup calls.
>
>
>> >>
>> 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.
>>
>
> Commit/Rollback is not a necessary condition, technically backup needs
> to be blocked only for the duration of a unlogged operation. It is
> just simpler to test and implement online backup if backup is
> unblocked on commit/rollback in these cases. To unblock immediately
> after the unlogged operation, I would need to go through all the code
> for unlogged operation and make sure unblock of backup is done at the
> right place and testing becomes more tricky.
>
> I believe blocking the backup until unlogged operations are committed
> is acceptable for now, because typically users will issue a
> commit/rollback after the unlogged operations are complete. User
> application is required to do anything special in this case, backup
> will just block for more time than ideally required. If some one finds
> this is not acceptable, online backup can be enhanced to block only
> for the duration of the unlogged operation at a later point of time.
>
>
> Thanks
> -suresh
>
>
>
>
>
>
>
>
>
>
Re: [jira] Commented: (DERBY-239) Need a online backup feature that
does not block update operations when online backup is in progress.
Posted by Suresh Thalamati <su...@gmail.com>.
Øystein Grøvlen wrote:
> Suresh,
>
> Thanks for considering my suggestions. A few responses below.
>
> --
> Øystein
>
> >> * 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?
>
Not a real backup call, but it related to the backup functionality and
disabling the log archive mode operation can not be rolled back, that
was the reason behind doing the implicit commit for this call also. I
will be removing the IDLE check and implicit commit in this case also
along with other backup calls.
> >> 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.
>
Commit/Rollback is not a necessary condition, technically backup needs
to be blocked only for the duration of a unlogged operation. It is
just simpler to test and implement online backup if backup is
unblocked on commit/rollback in these cases. To unblock immediately
after the unlogged operation, I would need to go through all the code
for unlogged operation and make sure unblock of backup is done at the
right place and testing becomes more tricky.
I believe blocking the backup until unlogged operations are committed
is acceptable for now, because typically users will issue a
commit/rollback after the unlogged operations are complete. User
application is required to do anything special in this case, backup
will just block for more time than ideally required. If some one finds
this is not acceptable, online backup can be enhanced to block only
for the duration of the unlogged operation at a later point of time.
Thanks
-suresh
Re: [jira] Commented: (DERBY-239) Need a online backup feature that
does not block update operations when online backup is in progress.
Posted by Øystein Grøvlen <Oy...@Sun.COM>.
Suresh,
Thanks for considering my suggestions. A few responses below.
--
Øystein
>>>>> "ST" == Suresh Thalamati <su...@gmail.com> 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
Re: [jira] Commented: (DERBY-239) Need a online backup feature that
does not block update operations when online backup is in progress.
Posted by Suresh Thalamati <su...@gmail.com>.
Hi Øystein,
Thanks for reviewing the online backup patches, my comments are
in-line for the issues , i did not respond in my earlier e-mail.
Øystein Grøvlen (JIRA) wrote:
> [ http://issues.apache.org/jira/browse/DERBY-239?page=comments#action_12361535 ]
>
> Øystein Grøvlen commented on DERBY-239:
> ---------------------------------------
>
> I have reviewed the latest patches (3 through6). (A bit late I must admit). Here are my comments:
>
> java/engine/org/apache/derby/catalog/SystemProcedures.java
>
> * SYSCS_BACKUP_DATABASE()
> - "By default this procedure will wait ..." Is it possible to
> change this behavior for this particular procedure? If not,
> "by default" is a bit misleading
> - "wait for the backup blocking unlogged operations to complete
> ..." is a bit heavy. I suggest just "wait for any unlogged
> operations to complete ..."
>
I will fix the comments.
> * SYSCS_ONLINE_BACKUP_DATABASE_AND_ENABLE_LOG_ARCHIVE_MODE()
> - Same comment on ONLINE as above
> - Could not boolean parameters be used now?
I am not sure boolean paramets work with stored procedures now.
Even if the support available, just to be consistent with existing
backup procedure parameters types, I think using INT is better for the
new procedures also.
>
> * 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.
Added this function to support implicit commit/rollback, This function
may not be needed any more. I will just call the low level backup
calls directly.
>
> * SYSCS_DISABLE_LOG_ARCHIVE_MODE()
> - Is checkBackupTransactionIsIdle() strictly necessary here? This
> seems like an operation where failures could be handle at
> statement level.
>
No. Idle check was necessary for issuing implicit commit to be
consistent with other backup calls.
>
> java/engine/org/apache/derby/iapi/store/access/AccessFactory.java
>
> * backup(String ...)
> - "Please see cloudscape on line ..." Derby?
>
> * backup(File ...)
> - I will just remind you of DERBY-665. I could do that work, but
> I am not sure it is a good idea for other people change the
> backup code while you are working on it. (May create merge
> conflicts for you.)
Thanks for reminding , I will fix it.
>
>
> java/engine/org/apache/derby/iapi/store/raw/xact/RawTransaction.java
>
> * setBackupBlockingState()
> - I do not like the name for this method. I suggest calling it
> blockBackup() or something like that. At least, the javadoc
> should explain what is meant by "backup blocking state".
>
Thanks for the suggestion , I will change the names to blockBackup()
and unblockBackup() .
>
> 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.
I can change them too :
blockBackupBlockingOperations()
unblockBackupBlockingOoperation()
Any suggestions ?
>
>
> java/engine/org/apache/derby/impl/db/BasicDatabase.java
>
> * backupAndEnableLogArchiveMode()
> - Non-standard indentation for parameters?
Not sure why I did that, will fix it.
>
>
> java/engine/org/apache/derby/impl/store/raw/RawStore.java
>
> * backup(String, boolean)
> - Typo in comment: "Check if there any backup ..." Remove "there"?
>
> * backupAndEnableLogArchiveMode(String, boolean)
> - Why do you need a finally clause? Would not a catch clause be
> sufficient? Then, you could eliminate the local 'error'
> variable.
if I catch on Throwable , may be I don't need finally in this case.
>
>
> java/engine/org/apache/derby/impl/store/raw/data/RFResource.java
>
> * add()/remove()
> - Are the casts to RawTransaction safe? Does this assumption have
> any impact on the modularity of the code?
Yes. RawTransaction is the one that implements Transaction, and the
way the Transaction is acquired it is sure to be RawTransaction.
Having said that I don't like the casting either , will look around
and see if there is a simple way to get RawTransaction directly.
>
> * serviceImmediately()
> - How is this change related to backup?
If the jar files are not removed on COMMIT , removal work gets
assigned to RawStoreDaemon(postcommit) thread. Backup of jars and
Removal of Jar can conflict. Blocking the RawStoreDaemon on backup may
not be a good idea , so change the removal of jars to happen on the
commit calls itself and also I think it better remove files on commit.
>
>
> 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?
May be backup can be allowed once the unlogged operation is complete,
I have not looked in-depth to allow immediately after the unlog
operations is complete, one case that I thought likely to have
problem is JAR file removal on post commit conflicting with backup
jar copy operation.
For now, I think enforcing the COMMIT restriction is ok.
>
>
> java/engine/org/apache/derby/impl/store/raw/xact/XactFactory.java
>
> * canStartBackupBlockingOperation()
> - Since this method now may wait for backup to complete, I do not
> feel canStartBackupBlockingOperation() is a good name.
> - Symmetric naming with backupBlockingOperationFinished() would
> make it more evident that these two functions should be called
> in pairs.
will change these name also too :
blockBackup() and unblockBackup()
>
>
> java/engine/org/apache/derby/loc/messages_en.properties
>
> * XSRSA.S
> - Suggest the following change:
> "Cannot backup the database when unlogged operations are uncommitted. Please commit the transactions with backup blocking operations or use the backup procedure with option to wait for them to complete."
> - Generally, I am not very fond of these long error messages. I
> think it would be better with just a single sentence, and then
> the user should be able to look up an explanation in a manual.
>
> * XSRSB.S
> - Suggest the following change:
> "Backup operation can not be performed in an active transaction. Please start a new transaction to execute backup procedures."
>
Thanks for the suggestion , I will update the error messages.
>
> java/testing/org/apache/derbyTesting/functionTests/tests/store/OnlineBackupTest1.java
>
> * runTest()
> - Should have more than one unlogged operation in parallel in
> order to fully test that the counting of unlogged operations
> work.
> - Suggest to add test that when doing backup in a non-idle
> transaction, previous work has not been rolled back when backup
> fails, and that one can continue with more operations within the
> same transaction.
>
> * runConsistencyChecker()
> - This does only check consistnecy of internal structures. Should
> also check consistency of application data. Maybe you could
> execute a select?
>
> * performDmlActions()
> - I assume the intention here is to do "while (!stopActivity)"
I will enhance the test to address the above mentioned cases.
>
> * endUnloggedAction()
> - What is the role of the insert? Is is unlogged? It is not
> evident from the method name or Javadoc why you are doing
> inserts here.
I intended to check if that insert will make it to backup.
>
> * select()
> - What is the point of doing consistency checks on columns that
> are not updated? If id and name does not match, that must be
> caused by errors in code that is not particular to backup.
>
u are right, I will change the select to check on columns that are
being updated.
>
> java/testing/org/apache/derbyTesting/functionTests/tests/store/OnlineBackupTest3.java
>
> * installJarTest()
> - Typos in comment: "// followng backup call should because jar
> operation is pending". Should what?
> - Comment say: "//Now commit the jar operation in connection1 for
> backup to proceed." The next statement does an insert. This is
> confusing.
> - It would be nice if the test checked that jar operation is still
> waiting for backup when create index has completed, but I guess
> this is a bit difficult to achieve.
>
> * A mix of tab and spaces for indentation. For new files that
> should not be necessary!
>
> * removeJarTest()
> - Comment copied from installJarTest?
> "// wait for customer app jar installation to finish now. "
I will fix them.
>
>
> java/testing/org/apache/derbyTesting/functionTests/tests/store/onlineBackupTest2.sql
>
> * Do you have idea of how frequently it happens that backup thread
> has not been blocked yet when backupdir is created?
In this test,If backup is blocking correctly on unlogged operations
backupdir should not get created.
>How long a
> sleep would you need to be certain?
>
I checked with 1000 msec sleep time, test failed on my laptop if
backup is not blocking correctly for the unlogged operations to commit.
Test will not fail even if sleep() is removed , it is just to make
sure there is a chance for the test to actually enter the backup
blocking code. I can not be certain how much sleep time will make
sure that will happen, it will depend on the hardware and OS on which
the test is being run.
Thanks
-suresht
>
>
>
>>Need a online backup feature that does not block update operations when online backup is in progress.
>>--------------------------------------------------------------------------------------------------------
>>
>> Key: DERBY-239
>> URL: http://issues.apache.org/jira/browse/DERBY-239
>> Project: Derby
>> Type: New Feature
>> Components: Store
>> Versions: 10.1.1.0
>> Reporter: Suresh Thalamati
>> Assignee: Suresh Thalamati
>> Attachments: obtest_customer.jar, onlinebackup.html, onlinebackup1.html, onlinebackup_1.diff, onlinebackup_2.diff, onlinebackup_3.diff, onlinebackup_4.diff, onlinebackup_5.diff, onlinebackup_6.diff
>>
>>Currently Derby allows users to perfoms online backups using SYSCS_UTIL.SYSCS_BACKUP_DATABASE() procedure, but while the backup is in progress, update operations are temporarily blocked, but read operations can still proceed.
>>Blocking update operations can be real issue specifically in client server environments, because user requests will be blocked for a long time if a
>>backup is in the progress on the server.
>
>
Re: [jira] Commented: (DERBY-239) Need a online backup feature that
does not block update operations when online backup is in progress.
Posted by Suresh Thalamati <su...@gmail.com>.
Thanks for the input Øystein, Mike. Now I am also convinced approach
#2 is better. As this is functional issue, I think it is better to
resolve this issue now than leave it for future. I will work on
submitting the patch with the second approach.
Thanks
-suresht
Mike Matrigali wrote:
> approach #2 seems more "online", if it doesn't cause other problems it
> seems better. In reality I don't think most users will see any
> difference as it seems wierd to on purpose be doing other things in
> a transaction and then execute a backup. Documentation should be clear
> that Derby cannot back out a backup.
>
> Suresh Thalamati wrote:
>
>> Thanks for taking time to review. My comments are in-line for some
>> of the questions, I will respond to the other questions in another
>> e-mail.
>>
>> From you comments, one issue that we are uable to come to conclusion,
>> is what to do if a backup call is issued in a transaction that already
>> has a pending unlogged operations.
>>
>> 1) Current approach:
>>
>> a) permit backup call only in a non-idle transaction.
>> b) issue a implicit commit/rollback after the backup call is done.
>>
>> 2) Other approch is :
>>
>> a) prevent backup calls only when a transaction already has executed
>> unlogged operations.
>> b) Don't issue implicit commit/rollback after the backup
>>
>> I am ok with either of these approaches. I would like to know if
>> anyone prefers one approach over the other.
>>
>>
>>
>> Øystein Grøvlen (JIRA) wrote:
>>
>>> [
>>> http://issues.apache.org/jira/browse/DERBY-239?page=comments#action_12361535
>>> ]
>>> Øystein Grøvlen commented on DERBY-239:
>>> ---------------------------------------
>>>
>>> I have reviewed the latest patches (3 through6). (A bit late I must
>>> admit). Here are my comments:
>>>
>>
>>>
>>> * SYSCS_ONLINE_BACKUP_DATABASE()
>>> - Since both backup procedures are ONLINE, it is a bit misleading
>>> to use this word to distinguish between the two backup
>>> procedures. I guess the main reason for choosing a new name is
>>> the extra parameter. In that case, I think it would be better
>>> to name the new procedure, SYSCS_BACKUP_DATABASE_NOWAIT and
>>> leave out the parameter.
>>> - The javadoc does not say what will happen if wait is 0. Will one
>>> get an exception if there is unlogged operations?
>>
>>
>>
>> I also think SYSCS_BACKUP_DATABASE_NOWAIT makes more sense, will
>> change the the procedure names with ONLINE to NO_WAIT, if no one else
>> has any objections.
>>
>> SYSCS_BACKUP_DATABASE_NOWAIT
>> SYSCS_ONLINE_BACKUP_DATABASE_AND_ENABLE_LOG_ARCHIVE_MODE_NO_WAIT
>>
>>
>>>
>>> * backupDatabase()
>>> - Is this the right layer for checking that the transaction is
>>> idle and for doing rollback/commit the transaction? Since this
>>> is a requirement for the logic at lower layers to work
>>> correctly, not something that is done because it is the
>>> desirable behavior of the system procedure, I feel that this
>>> should be done at a lower layer.
>>
>>
>>
>> I think so, because it is better to commit/rollback at the jdbc layer
>> level than in rawstore, becuase if we add anything in language area
>> for backup , it will surprise us.
>>
>> in non-idle transaction error case backup should not commit/rollback,
>> that is why I pushed the check to jdbc level and also checking for
>> transaction is IDLE is already exposed to JDBC for some other code.
>>
>> Other approach was to make rawstore throw the error and cacth the
>> error in the jdbc layer and decide to issue commit/rollback.
>> I generally don't like catching exception and then deciding what to
>> based on the SQLState, so I decided to check for the transaction
>> idle state in SystemProcedures.java
>>
>>> - I know when we discussed this isssue earlier, I agreed that
>>> checking that the transaction is idle was a good solution.
>>> However, thinking a bit more about this, I think it would be
>>> better to fail the transaction when unlogged operations have
>>> been performed by the same transaction. That would limit it to
>>> those who actually need to be affected, and it would
>>> significantly reduce the probability of someone ever
>>> experiencing this problem.
>>
>>
>>
>> It is very rare that some user stating a backup in a non-idle
>> transaction. If you think there is a possibilty of users executing
>> backup in non-idle transactions, then we can change the backup to
>> check only for non-logged cases as us suggested. But if
>> we follow this approach then we can commit/rollback after the backup.
>>
>>
>>> - I am not very fond of automatic commits like this.
>>
>>
>>
>> I think it was decided in earlier e-mails that backups need not be
>> transactional. So it is good to issue commit becuase we are sure
>> transaction is in idle state when backup was started.
>>
>> If this is
>>
>>> necessary, I think it would be better to require that backup is
>>> performed in autocommit mode. Then the implications would be
>>> more evident to the users and not catch someone by surprise.
>>
>>
>>
>>
>> I think forcing for the backup to work only on autocommit is likely to
>> break existing applications that are doing backup in autocommit=false
>> mode.
>>
>>>
>>>> Need a online backup feature that does not block update
>>>> operations when online backup is in progress.
>>>> --------------------------------------------------------------------------------------------------------
>>>>
>>>>
>>>> Key: DERBY-239
>>>> URL: http://issues.apache.org/jira/browse/DERBY-239
>>>> Project: Derby
>>>> Type: New Feature
>>>> Components: Store
>>>> Versions: 10.1.1.0
>>>> Reporter: Suresh Thalamati
>>>> Assignee: Suresh Thalamati
>>>> Attachments: obtest_customer.jar, onlinebackup.html,
>>>> onlinebackup1.html, onlinebackup_1.diff, onlinebackup_2.diff,
>>>> onlinebackup_3.diff, onlinebackup_4.diff, onlinebackup_5.diff,
>>>> onlinebackup_6.diff
>>>>
>>>> Currently Derby allows users to perfoms online backups using
>>>> SYSCS_UTIL.SYSCS_BACKUP_DATABASE() procedure, but while the backup
>>>> is in progress, update operations are temporarily blocked, but read
>>>> operations can still proceed.
>>>> Blocking update operations can be real issue specifically in client
>>>> server environments, because user requests will be blocked for a
>>>> long time if a backup is in the progress on the server.
>>>
>>>
>>>
>>>
>>
>>
>>
>
>
Re: [jira] Commented: (DERBY-239) Need a online backup feature that
does not block update operations when online backup is in progress.
Posted by Mike Matrigali <mi...@sbcglobal.net>.
approach #2 seems more "online", if it doesn't cause other problems it
seems better. In reality I don't think most users will see any
difference as it seems wierd to on purpose be doing other things in
a transaction and then execute a backup. Documentation should be clear
that Derby cannot back out a backup.
Suresh Thalamati wrote:
> Thanks for taking time to review. My comments are in-line for some
> of the questions, I will respond to the other questions in another e-mail.
>
> From you comments, one issue that we are uable to come to conclusion,
> is what to do if a backup call is issued in a transaction that already
> has a pending unlogged operations.
>
> 1) Current approach:
>
> a) permit backup call only in a non-idle transaction.
> b) issue a implicit commit/rollback after the backup call is done.
>
> 2) Other approch is :
>
> a) prevent backup calls only when a transaction already has executed
> unlogged operations.
> b) Don't issue implicit commit/rollback after the backup
>
> I am ok with either of these approaches. I would like to know if anyone
> prefers one approach over the other.
>
>
>
> Øystein Grøvlen (JIRA) wrote:
>
>> [
>> http://issues.apache.org/jira/browse/DERBY-239?page=comments#action_12361535
>> ]
>> Øystein Grøvlen commented on DERBY-239:
>> ---------------------------------------
>>
>> I have reviewed the latest patches (3 through6). (A bit late I must
>> admit). Here are my comments:
>>
>
>>
>> * SYSCS_ONLINE_BACKUP_DATABASE()
>> - Since both backup procedures are ONLINE, it is a bit misleading
>> to use this word to distinguish between the two backup
>> procedures. I guess the main reason for choosing a new name is
>> the extra parameter. In that case, I think it would be better
>> to name the new procedure, SYSCS_BACKUP_DATABASE_NOWAIT and
>> leave out the parameter.
>> - The javadoc does not say what will happen if wait is 0. Will one
>> get an exception if there is unlogged operations?
>
>
> I also think SYSCS_BACKUP_DATABASE_NOWAIT makes more sense, will
> change the the procedure names with ONLINE to NO_WAIT, if no one else
> has any objections.
>
> SYSCS_BACKUP_DATABASE_NOWAIT
> SYSCS_ONLINE_BACKUP_DATABASE_AND_ENABLE_LOG_ARCHIVE_MODE_NO_WAIT
>
>
>>
>> * backupDatabase()
>> - Is this the right layer for checking that the transaction is
>> idle and for doing rollback/commit the transaction? Since this
>> is a requirement for the logic at lower layers to work
>> correctly, not something that is done because it is the
>> desirable behavior of the system procedure, I feel that this
>> should be done at a lower layer.
>
>
> I think so, because it is better to commit/rollback at the jdbc layer
> level than in rawstore, becuase if we add anything in language area for
> backup , it will surprise us.
>
> in non-idle transaction error case backup should not commit/rollback,
> that is why I pushed the check to jdbc level and also checking for
> transaction is IDLE is already exposed to JDBC for some other code.
>
> Other approach was to make rawstore throw the error and cacth the error
> in the jdbc layer and decide to issue commit/rollback.
> I generally don't like catching exception and then deciding what to
> based on the SQLState, so I decided to check for the transaction idle
> state in SystemProcedures.java
>
>> - I know when we discussed this isssue earlier, I agreed that
>> checking that the transaction is idle was a good solution.
>> However, thinking a bit more about this, I think it would be
>> better to fail the transaction when unlogged operations have
>> been performed by the same transaction. That would limit it to
>> those who actually need to be affected, and it would
>> significantly reduce the probability of someone ever
>> experiencing this problem.
>
>
> It is very rare that some user stating a backup in a non-idle
> transaction. If you think there is a possibilty of users executing
> backup in non-idle transactions, then we can change the backup to check
> only for non-logged cases as us suggested. But if
> we follow this approach then we can commit/rollback after the backup.
>
>
>> - I am not very fond of automatic commits like this.
>
>
> I think it was decided in earlier e-mails that backups need not be
> transactional. So it is good to issue commit becuase we are sure
> transaction is in idle state when backup was started.
>
> If this is
>
>> necessary, I think it would be better to require that backup is
>> performed in autocommit mode. Then the implications would be
>> more evident to the users and not catch someone by surprise.
>
>
>
> I think forcing for the backup to work only on autocommit is likely to
> break existing applications that are doing backup in autocommit=false mode.
>
>>
>>> Need a online backup feature that does not block update operations
>>> when online backup is in progress.
>>> --------------------------------------------------------------------------------------------------------
>>>
>>>
>>> Key: DERBY-239
>>> URL: http://issues.apache.org/jira/browse/DERBY-239
>>> Project: Derby
>>> Type: New Feature
>>> Components: Store
>>> Versions: 10.1.1.0
>>> Reporter: Suresh Thalamati
>>> Assignee: Suresh Thalamati
>>> Attachments: obtest_customer.jar, onlinebackup.html,
>>> onlinebackup1.html, onlinebackup_1.diff, onlinebackup_2.diff,
>>> onlinebackup_3.diff, onlinebackup_4.diff, onlinebackup_5.diff,
>>> onlinebackup_6.diff
>>>
>>> Currently Derby allows users to perfoms online backups using
>>> SYSCS_UTIL.SYSCS_BACKUP_DATABASE() procedure, but while the backup
>>> is in progress, update operations are temporarily blocked, but read
>>> operations can still proceed.
>>> Blocking update operations can be real issue specifically in client
>>> server environments, because user requests will be blocked for a long
>>> time if a backup is in the progress on the server.
>>
>>
>>
>
>
>
Re: [jira] Commented: (DERBY-239) Need a online backup feature that
does not block update operations when online backup is in progress.
Posted by Suresh Thalamati <su...@gmail.com>.
Øystein Grøvlen wrote:
>>>>>>"ST" == Suresh Thalamati <su...@gmail.com> writes:
>
>
> ST> Thanks for taking time to review. My comments are in-line for some
> ST> of the questions, I will respond to the other questions in another
> ST> e-mail.
>
>
> ST> From you comments, one issue that we are uable to come to conclusion,
> ST> is what to do if a backup call is issued in a transaction that already
> ST> has a pending unlogged operations.
>
>
> ST> 1) Current approach:
>
> ST> a) permit backup call only in a non-idle transaction.
> ST> b) issue a implicit commit/rollback after the backup call is done.
>
> ST> 2) Other approch is :
>
> ST> a) prevent backup calls only when a transaction already has executed
> ST> unlogged operations.
>
> ST> b) Don't issue implicit commit/rollback after the backup
>
> ST> I am ok with either of these approaches. I would like to know if
> ST> anyone prefers one approach over the other.
>
> I prefer 2) for two reasons:
> - 2a) will impact less users than 1a). I agree that not many will
> be hit by any of them, but it is possible that someone may think
> of reading or recording information in the database as part of
> doing a backup. I think it is much less likely that someone
> will combine an unlogged operation with backup.
> - I think we should if possible avoid exceptions to standard
> behavior. Implicit commit/rollback is an exception to standard
> behavior. Such exceptions require specific documentation and
> makes the product more complex to use. Users tend not to read
> such documentation. 1a) is less of a problem than 1b) since the
> user will get an error if they are not aware of the problem.
> 1b) will not necessarily cause an error, but the transactional
> behavior of an application may be different from what the user
> thinks it is.
>
> That said, one can not use unlimited resources in order to get the
> perfect solution. If 2) is much more work than 1), I see the argument
> for doing 1) now and just file a JIRA issue for the better solution.
>
> >> * backupDatabase()
>
> >> - Is this the right layer for checking that the transaction is
> >> idle and for doing rollback/commit the transaction? Since this
> >> is a requirement for the logic at lower layers to work
> >> correctly, not something that is done because it is the
> >> desirable behavior of the system procedure, I feel that this
> >> should be done at a lower layer.
>
> ST> I think so, because it is better to commit/rollback at the jdbc layer
> ST> level than in rawstore, becuase if we add anything in language area
> ST> for backup , it will surprise us.
>
> Surprise who? In what way?
>
> ST> in non-idle transaction error case backup should not commit/rollback,
> ST> that is why I pushed the check to jdbc level and also checking for
> ST> transaction is IDLE is already exposed to JDBC for some other code.
>
>
> ST> Other approach was to make rawstore throw the error and cacth the
> ST> error in the jdbc layer and decide to issue commit/rollback.
>
> I do not understand why you need to catch the error. If I understand
> you correctly you just said that commit/rollback should not be done if
> the transaction is non-idle.
>
My understanding is right layer to issue implicit commit/rollbacks is
JDBC layer than store, because the transaction backup procedure is
using is setup from the jdbc layer.
With that assumption if check for the transaction is IDLE is done in
the store layer backup methods then if it is not IDLE then that info
has to passed back all the way to JDBC layer, so that backup procdure
does not issue a commit/rollback. If use the exception mechanism to
do that , then I have to check for Error state before issuing a
rollback, which I normally don't like to do. That is all the reason
behind doing check for transacion is IDLE before calling backup
methods in store.
if implicit commit/rollbacks are not done , then I agree with you the
right place do such checks is in the store layer.
> ST> I generally don't like catching exception and then deciding what to
> ST> based on the SQLState, so I decided to check for the transaction idle
> ST> state in SystemProcedures.java
>
Re: [jira] Commented: (DERBY-239) Need a online backup feature that
does not block update operations when online backup is in progress.
Posted by Øystein Grøvlen <Oy...@Sun.COM>.
>>>>> "ST" == Suresh Thalamati <su...@gmail.com> writes:
ST> Thanks for taking time to review. My comments are in-line for some
ST> of the questions, I will respond to the other questions in another
ST> e-mail.
ST> From you comments, one issue that we are uable to come to conclusion,
ST> is what to do if a backup call is issued in a transaction that already
ST> has a pending unlogged operations.
ST> 1) Current approach:
ST> a) permit backup call only in a non-idle transaction.
ST> b) issue a implicit commit/rollback after the backup call is done.
ST> 2) Other approch is :
ST> a) prevent backup calls only when a transaction already has executed
ST> unlogged operations.
ST> b) Don't issue implicit commit/rollback after the backup
ST> I am ok with either of these approaches. I would like to know if
ST> anyone prefers one approach over the other.
I prefer 2) for two reasons:
- 2a) will impact less users than 1a). I agree that not many will
be hit by any of them, but it is possible that someone may think
of reading or recording information in the database as part of
doing a backup. I think it is much less likely that someone
will combine an unlogged operation with backup.
- I think we should if possible avoid exceptions to standard
behavior. Implicit commit/rollback is an exception to standard
behavior. Such exceptions require specific documentation and
makes the product more complex to use. Users tend not to read
such documentation. 1a) is less of a problem than 1b) since the
user will get an error if they are not aware of the problem.
1b) will not necessarily cause an error, but the transactional
behavior of an application may be different from what the user
thinks it is.
That said, one can not use unlimited resources in order to get the
perfect solution. If 2) is much more work than 1), I see the argument
for doing 1) now and just file a JIRA issue for the better solution.
>> * backupDatabase()
>> - Is this the right layer for checking that the transaction is
>> idle and for doing rollback/commit the transaction? Since this
>> is a requirement for the logic at lower layers to work
>> correctly, not something that is done because it is the
>> desirable behavior of the system procedure, I feel that this
>> should be done at a lower layer.
ST> I think so, because it is better to commit/rollback at the jdbc layer
ST> level than in rawstore, becuase if we add anything in language area
ST> for backup , it will surprise us.
Surprise who? In what way?
ST> in non-idle transaction error case backup should not commit/rollback,
ST> that is why I pushed the check to jdbc level and also checking for
ST> transaction is IDLE is already exposed to JDBC for some other code.
ST> Other approach was to make rawstore throw the error and cacth the
ST> error in the jdbc layer and decide to issue commit/rollback.
I do not understand why you need to catch the error. If I understand
you correctly you just said that commit/rollback should not be done if
the transaction is non-idle.
ST> I generally don't like catching exception and then deciding what to
ST> based on the SQLState, so I decided to check for the transaction idle
ST> state in SystemProcedures.java
>> - I know when we discussed this isssue earlier, I agreed that
>> checking that the transaction is idle was a good solution.
>> However, thinking a bit more about this, I think it would be
>> better to fail the transaction when unlogged operations have
>> been performed by the same transaction. That would limit it to
>> those who actually need to be affected, and it would
>> significantly reduce the probability of someone ever
>> experiencing this problem.
ST> It is very rare that some user stating a backup in a non-idle
ST> transaction. If you think there is a possibilty of users executing
ST> backup in non-idle transactions, then we can change the backup to
ST> check only for non-logged cases as us suggested. But if
ST> we follow this approach then we can commit/rollback after the backup.
As I say above, even if it is rare, it is much more likely than
combining unlogged operations and backup.
>> - I am not very fond of automatic commits like this.
ST> I think it was decided in earlier e-mails that backups need not be
ST> transactional. So it is good to issue commit becuase we are sure
ST> transaction is in idle state when backup was started.
ST> If this is
>> necessary, I think it would be better to require that backup is
>> performed in autocommit mode. Then the implications would be
>> more evident to the users and not catch someone by surprise.
ST> I think forcing for the backup to work only on autocommit is likely to
ST> break existing applications that are doing backup in autocommit=false
ST> mode.
Good point. I rest my case for autocommit mode.
--
Øystein
Re: [jira] Commented: (DERBY-239) Need a online backup feature that
does not block update operations when online backup is in progress.
Posted by Suresh Thalamati <su...@gmail.com>.
Thanks for taking time to review. My comments are in-line for some
of the questions, I will respond to the other questions in another
e-mail.
From you comments, one issue that we are uable to come to conclusion,
is what to do if a backup call is issued in a transaction that already
has a pending unlogged operations.
1) Current approach:
a) permit backup call only in a non-idle transaction.
b) issue a implicit commit/rollback after the backup call is done.
2) Other approch is :
a) prevent backup calls only when a transaction already has executed
unlogged operations.
b) Don't issue implicit commit/rollback after the backup
I am ok with either of these approaches. I would like to know if
anyone prefers one approach over the other.
Øystein Grøvlen (JIRA) wrote:
> [ http://issues.apache.org/jira/browse/DERBY-239?page=comments#action_12361535 ]
>
> Øystein Grøvlen commented on DERBY-239:
> ---------------------------------------
>
> I have reviewed the latest patches (3 through6). (A bit late I must admit). Here are my comments:
>
>
> * SYSCS_ONLINE_BACKUP_DATABASE()
> - Since both backup procedures are ONLINE, it is a bit misleading
> to use this word to distinguish between the two backup
> procedures. I guess the main reason for choosing a new name is
> the extra parameter. In that case, I think it would be better
> to name the new procedure, SYSCS_BACKUP_DATABASE_NOWAIT and
> leave out the parameter.
> - The javadoc does not say what will happen if wait is 0. Will one
> get an exception if there is unlogged operations?
I also think SYSCS_BACKUP_DATABASE_NOWAIT makes more sense, will
change the the procedure names with ONLINE to NO_WAIT, if no one else
has any objections.
SYSCS_BACKUP_DATABASE_NOWAIT
SYSCS_ONLINE_BACKUP_DATABASE_AND_ENABLE_LOG_ARCHIVE_MODE_NO_WAIT
>
> * backupDatabase()
> - Is this the right layer for checking that the transaction is
> idle and for doing rollback/commit the transaction? Since this
> is a requirement for the logic at lower layers to work
> correctly, not something that is done because it is the
> desirable behavior of the system procedure, I feel that this
> should be done at a lower layer.
I think so, because it is better to commit/rollback at the jdbc layer
level than in rawstore, becuase if we add anything in language area
for backup , it will surprise us.
in non-idle transaction error case backup should not commit/rollback,
that is why I pushed the check to jdbc level and also checking for
transaction is IDLE is already exposed to JDBC for some other code.
Other approach was to make rawstore throw the error and cacth the
error in the jdbc layer and decide to issue commit/rollback.
I generally don't like catching exception and then deciding what to
based on the SQLState, so I decided to check for the transaction
idle state in SystemProcedures.java
> - I know when we discussed this isssue earlier, I agreed that
> checking that the transaction is idle was a good solution.
> However, thinking a bit more about this, I think it would be
> better to fail the transaction when unlogged operations have
> been performed by the same transaction. That would limit it to
> those who actually need to be affected, and it would
> significantly reduce the probability of someone ever
> experiencing this problem.
It is very rare that some user stating a backup in a non-idle
transaction. If you think there is a possibilty of users executing
backup in non-idle transactions, then we can change the backup to
check only for non-logged cases as us suggested. But if
we follow this approach then we can commit/rollback after the backup.
> - I am not very fond of automatic commits like this.
I think it was decided in earlier e-mails that backups need not be
transactional. So it is good to issue commit becuase we are sure
transaction is in idle state when backup was started.
If this is
> necessary, I think it would be better to require that backup is
> performed in autocommit mode. Then the implications would be
> more evident to the users and not catch someone by surprise.
I think forcing for the backup to work only on autocommit is likely to
break existing applications that are doing backup in autocommit=false
mode.
>
>>Need a online backup feature that does not block update operations when online backup is in progress.
>>--------------------------------------------------------------------------------------------------------
>>
>> Key: DERBY-239
>> URL: http://issues.apache.org/jira/browse/DERBY-239
>> Project: Derby
>> Type: New Feature
>> Components: Store
>> Versions: 10.1.1.0
>> Reporter: Suresh Thalamati
>> Assignee: Suresh Thalamati
>> Attachments: obtest_customer.jar, onlinebackup.html, onlinebackup1.html, onlinebackup_1.diff, onlinebackup_2.diff, onlinebackup_3.diff, onlinebackup_4.diff, onlinebackup_5.diff, onlinebackup_6.diff
>>
>>Currently Derby allows users to perfoms online backups using SYSCS_UTIL.SYSCS_BACKUP_DATABASE() procedure, but while the backup is in progress, update operations are temporarily blocked, but read operations can still proceed.
>>Blocking update operations can be real issue specifically in client server environments, because user requests will be blocked for a long time if a
>>backup is in the progress on the server.
>
>