You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2017/04/13 19:44:15 UTC

[Bug 60988] New: Make the pre-exit pause duration configurable

https://bz.apache.org/bugzilla/show_bug.cgi?id=60988

            Bug ID: 60988
           Summary: Make the pre-exit pause duration configurable
           Product: Apache httpd-2
           Version: 2.5-HEAD
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: Core
          Assignee: bugs@httpd.apache.org
          Reporter: ablaszka@amazon.com
  Target Milestone: ---

Created attachment 34912
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34912&action=edit
Patch

When piped loggers are used, errors logged during module initialization (in the
post-config phase) may not always be present in the error log when the httpd
master process exits. This leads to something like "httpd exited with status 1"
being written to stdout without any corresponding error messages to explain
what went wrong. This occurs because there's a race condition between apache
exiting and piped loggers flushing the log entries to disk.

Apache waits 10ms before exiting to account for this, but that may not always
be enough. To fix this, this change makes the pre-exit pause duration
configurable. The current 10ms duration is still used by default but the
PRE_EXIT_PAUSE_TIME environment variable can be used to override that value.
ap_timeout_parameter_parse is used to parse the environment variable contents
allowing a flexible way to express the desired duration. If the value is too
large (would lead to under/overflow), < 0, or otherwise invalid, it is ignored
and the default value is used.

Git-generated patch (created via git-format-patch attached) from the tip of
trunk is attached. Including the diff here as well:


--- httpd_current/server/main.c 2017-04-13 19:35:01.000000000 +0000
+++ httpd_new/server/main.c     2017-04-13 17:11:57.000000000 +0000
@@ -260,25 +260,49 @@
 #endif
 }

-#define TASK_SWITCH_SLEEP 10000
+#define DEFAULT_PRE_EXIT_PAUSE_TIME 10000
+
+/*
+ * Pauses before exiting by sleeping for a configurable amount of time,
+ * defaulting to DEFAULT_PRE_EXIT_PAUSE_TIME. The goal is to allow any
in-flight
+ * log entries to be flushed to disk before exiting. Failure to do so can
result
+ * in some of the entries being lost. This is of particular importance for
piped
+ * loggers, which are outside of our control and can take an arbitrarily long
+ * time to complete. The default is intended to at least cause a context
switch
+ * to give the other process(es) time. If that isn't enough, the
+ * PRE_EXIT_PAUSE_TIME environment variable can be used to configure a longer
+ * pause time.
+ */
+static void pre_exit_pause(void)
+{
+    apr_interval_time_t sleep_time;
+    const char *sleep_time_env;
+
+    sleep_time = DEFAULT_PRE_EXIT_PAUSE_TIME;
+    sleep_time_env = getenv("PRE_EXIT_PAUSE_TIME");
+
+    if (sleep_time_env) {
+        if (ap_timeout_parameter_parse(sleep_time_env, &sleep_time, "ms") !=
APR_SUCCESS) {
+            sleep_time = DEFAULT_PRE_EXIT_PAUSE_TIME;
+        }
+    }
+
+    if (sleep_time < 0) {
+        sleep_time = DEFAULT_PRE_EXIT_PAUSE_TIME;
+    }
+
+    apr_sleep(sleep_time);
+}

 static void destroy_and_exit_process(process_rec *process,
                                      int process_exit_value)
 {
-    /*
-     * Sleep for TASK_SWITCH_SLEEP micro seconds to cause a task switch on
-     * OS layer and thus give possibly started piped loggers a chance to
-     * process their input. Otherwise it is possible that they get killed
-     * by us before they can do so. In this case maybe valueable log messages
-     * might get lost.
-     */
-
     /* If we are to print an error, we need the name before we destroy pool.
      * short_name is a pointer into argv, so remains valid.
      */
     const char *name = process->short_name ? process->short_name : "httpd";

-    apr_sleep(TASK_SWITCH_SLEEP);
+    pre_exit_pause();
     ap_main_state = AP_SQ_MS_EXITING;
     apr_pool_destroy(process->pool); /* and destroy all descendent pools */
     apr_terminate();

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 60988] Make the pre-exit pause duration configurable

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60988

Christophe JAILLET <ch...@wanadoo.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34912|0                           |1
           is patch|                            |
  Attachment #34912|application/mbox            |text/plain
          mime type|                            |

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 60988] Make the pre-exit pause duration configurable

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60988

Christophe JAILLET <ch...@wanadoo.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org