MySQL Bugs: #38781: Error handling and trace logging mixed in Maria

Bug #38781	Error handling and trace logging mixed in Maria
Submitted:	13 Aug 2008 20:32	Modified:	18 Mar 2010 10:40
Reporter:	Marc ALFF	Email Updates:
Status:	Won't fix	Impact on me:	None
Category:	MySQL Server: Maria storage engine	Severity:	S2 (Serious)
Version:		OS:	Any
Assigned to:	Marc ALFF	CPU Architecture:	Any

Description:
The code in the Maria storage engine use the following API:
ma_message_end_user
to print messages to the server logs.

The implementation is internally:
- raising errors with my_error()
- implementing special cases in the server error handling code,
in my_message_sql().

There are several issues with this design:
"errors" raised by ma_message_end_user are *not* SQL conditions,
and as such:
- should not be intercepted by DECLARE HANDLER in stored procedures,
- don't have a proper SQL_STATE

How to repeat:
Design issue, found by code review.

See ME_JUST_INFO, ME_JUST_WARNING
in my_message_sql().

Suggested fix:
Either print to the server log directly, or expose an interface to the storage
engines to do so, but do *not* overload the SQL condition handling mechanism
(my_error / my_message_sql) with out-of-bound data that does not belong here.

Found during the implementation of SIGNAL.

It's a minor thing, ma_message_no_user() can never be called from a stored routine, so it cannot break SP SQL handlers.

Hello Marc. I agree with Serg:
- ma_message_no_user() is called only out of a connection: it's called by Maria's Recovery to print its progress reports (and that happens before the server starts listening to connection requests), and by Maria's background checkpoint (which is a Maria-specific thread which takes checkpoints regularly).
- so it is never called by a connected client, neither by a SP.
That's why I agree that this is not urgent.
There may be a better way to do it (low-priority); for example, InnoDB merely has hard-coded sql_print_error() calls. We considered using sql_print_error in Maria too, but we chose to make ma_message_no_user() instead, the logic was:
- my_error() calls sql_print_error() when there's no THD, so my_error() is as good as sql_print_error() for our use case ("it does the job")
- my_error() can choose the message for the user's language (via sql/share/errmsg.txt) which sql_print_error() in engines does not do.

see also http://forge.mysql.com/worklog/task.php?id=2940

Thank you for taking the time to write to us, but this is not a bug; See previous comments for the bug

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/55133

2837 Marc Alff	2008-10-02
      Bug#38781 (Error handling and trace logging mixed in Maria)
      
      --- Proof of concept patch, not ready for production ---
      
      This patch removes the change implemented in the server error handling
      code (my_message_sql) for Maria, and remove the use of the following flags:
      - ME_JUST_INFO
      - ME_JUST_WARNING
      
      The design constraints applicable to the problem are:
      - the Maria code can not have a static dependency on mysql
      - the Maria code needs to print to the server log files (when embedded)
      - the Maria code needs to print to stderr (when not embedded)
      - the server needs to be independend of Maria logic in my_message_sql
      
      The solution consists of:
      - defining a Plugin -> Server interface, usable by every plugin.
        This is a general need.
      - Provide this server interface to a plugin at initialization.
      - For embedded code (such as unit tests or stand alone binaries like
        maria_chk), provide an alternate implementation of this server interface.
      
      This patch was written as a technical proposal to resolve bug#38781,
      and server as a base for a technical discussion.
      It illustrates how:
      - dependencies to the server can be abstracted in a storage engine
      - a plugin->server interface can be implemented.

As said before, the original code is correct and should not be changed.

The patch is meaningless as Maria should use the my_error() system to report errors/warnings and not some other interface.

The reason for this is that all code that Maria uses in MySQL already uses this system and the proposed patch splits the logic of error/warnings messages so that some errors are reported one way and others are reported another way which is unacceptable.