maria-developers team mailing list archive

["Vladislav Vaintroub" <wlad@xxxxxxxxxxxxxxxx>]

Hi,
this broke Windows build. Would it be possible to fix it? 

When looking at the patch, I found some things that IMO could be done
better.

1. 
mysys/my_logger.c

extern char *mysql_data_home;
extern PSI_mutex_key key_LOCK_logger_service;

I think referencing variables that do not belong to mysys in mysys is a
layering violation. Can mysys be used without sql or not here. Shall
everything that links to mysys link  sql as well?

I think - If there is a need to store data home and PSI key from sql layer,
then this something can be passed via initialization function, rather than
used directly.
Alternatively: Is the functionality provided by my_logger required in mysys?
If not, why it is be there, and can it be moved to where it is used, e.g
into sql?

2.
LOGGER_HANDLE *logger_open(const char *path,
                           unsigned long size_limit,
                           unsigned int rotations)

Is (general case) 32 bit size limit limitation by design ? IF so, why is not
"int" used (or int32 if one wants a fancy typename)? 
Files can be larger than 4GB. Even on 32 bits,  and on 64 bit systems with
32 bit longs.  

Can this be fixed?

Thanks,
Wlad

> -----Original Message-----
> From: maria-developers-
> bounces+wlad=montyprogram.com@xxxxxxxxxxxxxxxxxxx [mailto:maria-
> developers-bounces+wlad=montyprogram.com@xxxxxxxxxxxxxxxxxxx] On Behalf
> Of Sergei Golubchik
> Sent: Dienstag, 13. März 2012 19:09
> To: maria-developers@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Maria-developers] MDEV-15 Log all SQL errors. in
> file:///home/hf/wmar/mdev15/
> 
> Hi, Holyfoot!
> 
> Just two comments, see below.
> Please fix and push ASAP!
> 
> On Mar 13, holyfoot@xxxxxxxxxxxx wrote:
> >
> > ------------------------------------------------------------
> > revno: 3320
> > revision-id: holyfoot@xxxxxxxxxxxx-20120313160142-ljcsacc11zb34uu6
> > parent: sergii@xxxxxxxxx-20120309082045-iyihz9ysqo82qb71
> > committer: Alexey Botchkov <holyfoot@xxxxxxxxxxxx>
> > branch nick: mdev15
> > timestamp: Tue 2012-03-13 20:01:42 +0400
> > message:
> >   MDEV-15 Log all SQL errors.
> >           The logger service added to provide the rotating log
functions.
> >           The SQL_ERROR_LOG plugin created to log the SQL errors usint
that
> logger service.
> 
> > === added file 'include/mysql/service_logger.h'
> > --- a/include/mysql/service_logger.h	1970-01-01 00:00:00 +0000
> > +++ b/include/mysql/service_logger.h	2012-03-13 16:01:42 +0000
> > +
> > +typedef struct logger_service_file_st LOGGER_SERVICE_FILE;
> 
> small comment: please, rename to LOGGER_HANDLE or
> LOGGER_DESCRIPTOR,
> or, say, MYSQL_LOGGER. The point is to avoid the word "FILE", because,
> in principle, it may log to syslog or to a table or whatever.
> (and better to avoid SERVICE too, because 1) this structure is not a
> service, it's a handle, 2) services are supposed to be transparent and
> invisible, they pretend to be regular function calls)
> 
> > +maria_declare_plugin(sql_errlog)
> > +{
> > +  MYSQL_AUDIT_PLUGIN,
> > +  &descriptor,
> > +  "SQL_ERROR_LOG",
> > +  "Alexey Botchkov",
> > +  "Log SQL level errors to a file with rotation",
> > +  PLUGIN_LICENSE_GPL,
> > +  sql_error_log_init,
> > +  sql_error_log_deinit,
> > +  0x0100,
> > +  NULL,
> > +  vars,
> > +  NULL,
> > +  0
> 
> Fix this, please. Two last values are - version (as a string), and
> maturity. For example,
> 
>    "1.0",
>    MariaDB_PLUGIN_MATURITY_ALPHA
> 
> > +}
> > +maria_declare_plugin_end;
> 
> Regards,
> Sergei
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~maria-developers
> Post to     : maria-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~maria-developers
> More help   : https://help.launchpad.net/ListHelp