>>>>> "ST" == Suresh Thalamati <[EMAIL PROTECTED]> 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
