Developers of Sympa

[IKEDA Soji <address@concealed>]

On Wed, 02 Oct 2013 12:11:40 +0200
David Verdin <address@concealed> wrote:

> Dear all,
> 
> Le 02/10/13 10:09, IKEDA Soji a e'crit :
> > Hi,
> >
> > On Thu, 19 Sep 2013 17:35:16 +0200
> > David Verdin <address@concealed> wrote:
> >
> >>         Logs
> >>
> >>   * We will get rid of interpolation in do_log calls (replace
> >>     Sympa::Log::do_log('err', "Could not open $file"); by
> >>     Sympa::Log::do_log('err', 'Could not open %s', $file);)
> >>   * We will inspect logs to distinguish, amongst the different "debug"
> >>     levels, which are relevant to end users and which are not. The first
> >>     will be put in log_message calls, the former in log_trace_message
> >>     calls. There is a problem with this simplification: some functions
> >>     are called very often and, if all debug logs are actually printed,
> >>     in an instance with a big activity, the performances will severely
> >>     decrease. It can even stall the server. So this simple dichotomy
> >>     between developpers and users could be problematic.
> >>   * 'warn' log level is useless. Let's have only two levels: "info" and
> >>     "err". That's enough.
> >>   * Sympa::Log::Database is aimed at providing web-readable logs for
> >>     list owners. It logs only main events to make them available through
> >>     Sympa web interface.
> >>   * the web specific logs are useful because they provide informations
> >>     specific to the web. It is important when analyzing logs. All three
> >>     logging functions have their rational. I don't know how to simplify
> >>     this part of the code.
> > I'm not confident of unusefulness of 'warn' level, while by now I
> > can also not prove usefulness of it.  So I'll withdraw it.
> >
> > On 'debug' level, I don't worry about performance.  Even when logging
> > is disabled inside log_message() (perhaps because debugging level is
> > low enough), this function shall be called.  When it was enabled,
> > decrese of performance is the result of configuration done by
> > administrator.
> >
> > Database logging looks to hit performance much for me.  It might
> > be disabled by a configuration parameter.
> Actually, database logs only usage is to let list owners query the logs.
> I don't think this functionnality is much used. We could simply get rid
> of database logging and replace it by simple queries inside filesystem logs.
> The problem would be to query rotated logs. Most system have daily log
> files archived in a way or another. There are a lot of different
> formalisms and It would be hard to find the relevant files.
> Still another solution: Replace relational database logs by filesystem
> database logs, and handle logs rotation inside Sympa, with a
> well-defined formalism.

I wrote about OTRS some weeks ago.  When OTRS writes logs to
backend (File/Syslog), it caches same things into shared memory
using internal format.  GUI reads the cache from shmem and shows it.

  https://github.com/OTRS/otrs/blob/rel-3_2/Kernel/System/Log.pm
  https://github.com/OTRS/otrs/blob/rel-3_2/Kernel/Modules/AdminLog.pm

It looks interesting, although I don't know whether it is
applicable to Sympa or not.


> >> Exception handling seems not to be resolved. the fatal_err calls were 
> >> usefull because they specified explicitely that Sympa should not keep on 
> >> running under certain circumstances. This is a clear distinction with 
> >> the return undef we used as exception handling.
> >> I am under the impression that all return undef could be replaced by 
> >> croak calls, providing we intercept exceptions at the right place and we 
> >> use the Carp option allowing to produce stak traces.
> >> But, we need to distinguish the very few circumstances in which sympa 
> >> should stop running, so that the daemons will stop gracefully. 
> >> Inspecting the code, it looks like all fatal_err calls were made in the 
> >> main daemons, so I think it should be easy.
> > My concern on exception:
> >
> > - It tends to skip clean-up processes inside "try" and other blocks.
> >   Not only skipping each logic, it is likly to cause leaks of
> >   rosources such as memory.  The function corresponding to "finally"
> >   may probably ease such situation, while it can worsen the next
> >   problem.
> If the "finally" resolves the memory leaks, I violently support this option.
> >
> > - It is a bit hard to read.  The idiom using eval {} and if () is
> >   obvious (additionally, by this idiom, programmers must re-throw
> >   uncaught exceptions by themselves).  Syntax sugers provided by
> >   several modules are not always acceptable by average Perl
> >   programmers.
> If we set up clear rules about how exceptions are handled in Sympa on
> the Sympa web site, it should not be too much trouble.

D'accord, I'll try to figure out the rule.

IMO to emulate "try-catch" by Perl5, objects will be passed to
croak, instead of string.  Here is a template:

    # "try" block
    eval {

        # *1

        # Throw exception if error occurred
        if (...) {
            # *3
            croak Sympa::Exception::Foo->new(
                File => __FILE__, Line => __LINE__, Message => 'error 
occurred'
            );
        }

        # *2

    };

    # "catch" block
    if (ref $@ eq 'Sympa::Exception::Foo') {

        # process exception

    } elsif (ref $@ eq 'Sympa::Exception::Bar') {

        # process another exception

    }
    # uncuaght exceptions
    # We have responsibility to re-throw exceptions we won't catch.
    elsif (ref $@) {
        croak $@;
    } elsif ($@) {
        croak "$@\n";
    }

Last two elsif blocks are somewhat eyesore, but they are exactly
necessary not to miss exceptions raised by others (maybe in lower
level).


My first concern: For example, a filehandle is opened at (*1) then
closed at (*2).  When exception is thrown, that filehandle can
leak.  So we must also close it at (*3).  It is not easy to compile
this clean-up process into single place such as "finally" block.

# Here, I ignored the alternative to use CPAN modules.

Anyway, my second concern: is the rule above easy enough to keep?
(this is a simple question, not rhetorical)


> > I'm not an opponent of exception, contrary, I prefer to this
> > concept.  But by now I'm undecided.
> There is something still unclear to me: How exceptions are escalated. If
> I understand correctly, we plan to croak or carp everywhere we used
> "return undef" and eval{} these exceptions at the very top level (i.e.,
> in each Sympa daemon loops). In the end, will we have only the original
> message or all the croak messages we will meet between the original
> error and the final eval{} interception?

"try-catch" structure described above may appear in several levels,
because clean-up processes such as filehandle can be accomplished
only in each level.

Each "try-catch" catches particular exceptions and processes it ---
logs messages, recovers errors etc.  Uncaught exceptions will be
escalated (re-thrown) to upper level.  Once an exception has been
caught, it no longer will propagate to upper level.

Finally at uppermost level, Perl catches exceptions never caught
and dies.


There is one more comment on implementation.

Error messages are contained in exception objects.  Also,
information of call stack should be collected when each exception
object is instantiated.  These information may be shown as error
message and traceback.
However, if exception is simple scalar (thrown by external modules or
Perl), traceback cannot be shown.  Because information of call stack
had been lost.


Regards,

--- Soji


-- 
株式会社 コンバージョン  セキュリティ&OSSソリューション部   池田荘児
〒231-0004　神奈川県横浜市中区元浜町3-21-2 ヘリオス関内ビル7F
e-mail address@concealed  TEL 045-640-3550
http://www.conversion.co.jp/