Chuck,
You basically agreed to go for my patch but I would still like to know that you
understand my reasons behind the different solution.
Chuck Bell wrote:
> I don't understand: "a) and b), as they are not (will be not) available
> during the process" -- how is that possible? Even if we decouple the logger
> class from the server we will still need to communicate with the server's
> error handling code.
This is the main problem I'm trying to solve. Destinations a) and b) are the
error code/message returned from BACKUP/RESTORE command and client's
error/warning stack which is populated while server executes client's SQL
statement.
When BACKUP/RESTORE is asynchronous, these destinations are not available for
the whole duration of the backup/restore process. Imagine this: DBA connects to
server and issues "BACKUP DATABASE foo TO 'foo.bak'". The statement returns
immediately and successfully, returning backup id. Then DBA can even close his
connection to the server and the backup operation will continue in the
background. Thus, obviously, any errors or warnings issued while backup is
running can not be reported as a result of the BACKUP statement (which has
already returned) or pushed on the client's error/warning stack (as client has
disconnected).
This is why the Logger class should not report errors via my_error() or
push_warning() calls - the logger instance will exists during whole time of
backup/restore operation and will be used to report errors/warnings at any
moment - possibly also when there is no client connection any more.
For this reason, I want to separate generating the result of BACKUP/RESTORE
statements (whether it is an error or a valid backup_id result) from logging
backup related error/warnings. The idea is that the backup logging mechanism, in
the form of the Logger class, will be designed as we want it to work in the
future, while handling of BACKUP/RESTORE result might change as needed.
For example, now the BACKUP/RESTORE are synchronous and therefore they can and
should report all errors/warning reported during the operation. But in the
future they will be asynchronous and thus will not do it any more. I want to
change the way BACKUP/RESTORE report errors, but not the way Logger works.
And this is what my patch does. It assumes that Logger works as it will work in
the future, that is, not report errors to the client. Instead, all reported
errors and warnings are stored inside the logger and then, when the
backup/restore operation is finished, the code which is responsible for sending
response to the client (i.e. execute_backup_command() function) iterates over
stored errors/warnings, pushes them on the client's stack and returns error
code/message if appropriate.
Rafal
I think this abstraction is too complex and we need not
> go so far. What is the goal of such complexity? What precept of OOD are we
> trying to achieve? Devil's advocate question: Why should a logger for online
> backup not log warnings and errors and pass them to the server?
>
> </commentary>
>
> Chuck
>
