[GitHub] [openwhisk] markusthoemmes commented on a change in pull request #4582: Action container log collection does not wait for sentinel on developer error

[GitBox <gi...@apache.org>]

markusthoemmes commented on a change in pull request #4582: Action container log collection does not wait for sentinel on developer error
URL: https://github.com/apache/openwhisk/pull/4582#discussion_r313241590
 
 

 ##########
 File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainer.scala
 ##########
 @@ -257,17 +257,43 @@ class DockerContainer(protected val id: ContainerId,
    * previous activations that have to be skipped. For this reason, a starting position
    * is kept and updated upon each invocation.
    *
-   * If asked, check for sentinel markers - but exclude the identified markers from
-   * the result returned from this method.
+   * There are two possible modes controlled by parameter waitForSentinel:
    *
-   * Only parses and returns as much logs as fit in the passed log limit.
+   * 1. Wait for sentinel: tail container log file until two sentinel markers show up. Complete
+   *                       once two sentinel markers have been identified, regardless whether more
+   *                       data could be read from container log file.
+   *                       A log file reading error is reported if sentinels cannot be found.
+   *                       Managed action runtimes use the the sentinels to mark the end of
+   *                       an individual activation.
+   *
+   * 2. Do not wait for sentinel: read container log file up to its end. Stop reading once the end
+   *                              has been reached. Complete once two sentinel markers have been
+   *                              identified, regardless whether more data could be read from
+   *                              container log file.
+   *                              No log file reading error is reported if sentinels cannot be found.
+   *                              Blackbox actions do not necessarily produce marker sentinels properly,
+   *                              so this mode is used for all blackbox actions.
+   *                              In addition, this mode can / should be used in error situations with
+   *                              managed action runtimes where sentinel markers may be missing or
+   *                              arrive too late - Example: action exceeds time or memory limit during
+   *                              init or run.
+   *
+   * The result returned from this method does never contain any log sentinel markers. These are always
+   * filtered - regardless of the specified waitForSentinel mode.
+   *
+   * Only parses and returns as much logs as fit in the passed log limit. Stops log collection with an error
+   * if processing takes too long or time gaps between processing individual log lines are too long.
    *
    * @param limit the limit to apply to the log size
    * @param waitForSentinel determines if the processor should wait for a sentinel to appear
-   *
    * @return a vector of Strings with log lines in our own JSON format
    */
   def logs(limit: ByteSize, waitForSentinel: Boolean)(implicit transid: TransactionId): Source[ByteString, Any] = {
+    // Define a time limit for collecting and processing the logs of a single activation.
+    // If this time limit is exceeded, log processing is stopped and declared unsuccessful.
+    // Allow 2 seconds per MB log limit, with a minimum of 10 seconds.
+    val logProcessingTimeout = limit.toMB.toInt.max(5) * 2.seconds
 
 Review comment:
   This is somewhat arbitrary and might need some tweaking. Can you run this code through a loadtest to determine we don't hit this overall timeout under load too often?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services