Hi Lars,

On Mon, Oct 04, 2010 at 08:23:17PM +0200, Lars Ellenberg wrote:
> # HG changeset patch
> # User Lars Ellenberg <[email protected]>
> # Date 1286216544 -7200
> # Node ID b59be1e5279175464c3afb1b467531580f49da8b
> # Parent  0285b706fcde4d989291ab845ed245f1c366bf1e
> Medium: add fflush back to cl_log
> 
> diff -r 0285b706fcde -r b59be1e52791 include/clplumbing/cl_log.h
> --- a/include/clplumbing/cl_log.h     Tue Sep 28 19:10:38 2010 +0200
> +++ b/include/clplumbing/cl_log.h     Mon Oct 04 20:22:24 2010 +0200
> @@ -34,6 +34,11 @@
>  #define      DEBUGPKT        (debug_level >= 4)
>  #define      DEBUGPKTCONT    (debug_level >= 5)
>  
> +/* no_fflush and do_fflush as optimization for the logging daemon,
> + * so it may buffer a few message lines, then fflush them out in one write.
> + * set do_fsync != 0, if you even want it to fsync. */
> +void         cl_direct_log_no_fflush(int priority, const char* buf, 
> gboolean, const char*, int, TIME_T);
> +void         cl_direct_log_do_fflush(int do_fsync);
>  void         cl_direct_log(int priority, const char* buf, gboolean, const 
> char*, int, TIME_T);
>  void            cl_log(int priority, const char * fmt, ...) 
> G_GNUC_PRINTF(2,3);
>  void            cl_perror(const char * fmt, ...) G_GNUC_PRINTF(1,2);
> diff -r 0285b706fcde -r b59be1e52791 lib/clplumbing/cl_log.c
> --- a/lib/clplumbing/cl_log.c Tue Sep 28 19:10:38 2010 +0200
> +++ b/lib/clplumbing/cl_log.c Mon Oct 04 20:22:24 2010 +0200
> @@ -522,10 +522,15 @@
>   * non-blocking IPC.
>   */
>  
> +/* As performance optimization we keep the file-handle
> + * open all the time */
> +static FILE * log_fp = NULL;
> +static FILE * debug_fp = NULL;
> +
>  /* Cluster logging function */
> -void
> -cl_direct_log(int priority, const char* buf, gboolean use_priority_str,
> -           const char* entity, int entity_pid, TIME_T ts)
> +static void
> +cl_direct_log_(int priority, const char* buf, gboolean use_priority_str,
> +           const char* entity, int entity_pid, TIME_T ts, int do_fflush)
>  {
>       const char *    pristr;
>       int     needprivs = !cl_have_full_privs();
> @@ -558,27 +563,25 @@
>       }
>  
>       if (debugfile_name != NULL) {
> -             static FILE * debug_fp = NULL;
>               if (!debug_fp) {
> -                     /* As performance optimization we keep the file-handle
> -                      * open all the time */
>                       debug_fp = open_log_file(debugfile_name);
>               }
> -             if (debug_fp)
> -                     append_log(debug_fp ,entity, entity_pid, ts, pristr, 
> -                                buf);
> +             if (debug_fp) {
> +                     append_log(debug_fp ,entity, entity_pid, ts, pristr, 
> buf);
> +                     if (do_fflush)
> +                             fflush(debug_fp);
> +             }
>       }
>  
>       if (priority != LOG_DEBUG && logfile_name != NULL) {
> -             static FILE * log_fp = NULL;
>               if (!log_fp) {
> -                     /* As performance optimization we keep the file-handle
> -                      * open all the time */
>                       log_fp = open_log_file(logfile_name);
>               }
> -             if (log_fp)
> -                     append_log(log_fp ,entity, entity_pid, ts, pristr, 
> -                                buf);
> +             if (log_fp) {
> +                     append_log(log_fp ,entity, entity_pid, ts, pristr, buf);
> +                     if (do_fflush)
> +                             fflush(log_fp);
> +             }
>       }
>  
>       if (needprivs) {
> @@ -588,7 +591,35 @@
>       return;
>  }
>  
> +void
> +cl_direct_log(int priority, const char* buf, gboolean use_priority_str,
> +           const char* entity, int entity_pid, TIME_T ts)
> +{
> +     cl_direct_log_(priority, buf, use_priority_str,
> +                   entity, entity_pid, ts, 1);
> +}
>  
> +void
> +cl_direct_log_no_fflush(int priority, const char* buf, gboolean 
> use_priority_str,
> +           const char* entity, int entity_pid, TIME_T ts)
> +{
> +     cl_direct_log_(priority, buf, use_priority_str,
> +                   entity, entity_pid, ts, 0);
> +}
> +
> +void cl_direct_log_do_fflush(int do_fsync)
> +{
> +     if (log_fp) {
> +             fflush(log_fp);
> +             if (do_fsync)
> +                     fsync(fileno(log_fp));
> +     }
> +     if (debug_fp) {
> +             fflush(debug_fp);
> +             if (do_fsync)
> +                     fsync(fileno(debug_fp));
> +     }
> +}
>  
>  /*
>   * This function can cost us realtime unless use_logging_daemon
> diff -r 0285b706fcde -r b59be1e52791 logd/ha_logd.c
> --- a/logd/ha_logd.c  Tue Sep 28 19:10:38 2010 +0200
> +++ b/logd/ha_logd.c  Mon Oct 04 20:22:24 2010 +0200
> @@ -750,10 +750,9 @@
>  static gboolean
>  direct_log(IPC_Channel* ch, gpointer user_data)
>  {
> -     
>       IPC_Message*            ipcmsg;
>       GMainLoop*              loop;
> -     
> +     int                     pri = LOG_DEBUG + 1;
>  
>       loop =(GMainLoop*)user_data;
>       
> @@ -791,7 +790,9 @@
>                       ,       copy.use_pri_str
>                       ,       copy.entity, copy.entity_pid
>                       ,       copy.timestamp);
> -             
> +
> +                     if (copy.priority < pri)
> +                             pri = copy.priority;
>  
>                       (void)logd_log;
>  /*
> @@ -810,6 +811,8 @@
>               }
>               
>       }
> +     cl_direct_log_do_fflush(pri <= LOG_ERR);
> +
>       if(needs_shutdown) {
>               cl_log(LOG_INFO, "Exiting write process");
>               g_main_quit(loop);

I like the patch. The idea with flush depending on priority is
good.

Unfortunately, there's a queue of patches for cl_log which I
didn't find time yet to review. They were posted by Bernd
Schubert to the pacemaker ML more than a month ago. I'm afraid
I've been doing a bad job lately :(

Also, Keisuke-san has concerns about that latest patch (also by
Bernd) which did make it into the glue repository. See the
bugzilla report at

http://developerbugs.linux-foundation.org/show_bug.cgi?id=2470

I think that one of the later patches undoes that direct logging
patch, at least in part. 

Anyway, I'll review the Bernd's series today or tomorrow.

Cheers,

Dejan


> _______________________________________________________
> Linux-HA-Dev: [email protected]
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to