You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by ssachin3108 <gi...@git.apache.org> on 2017/08/18 08:48:25 UTC
[GitHub] logging-log4net pull request #15: https://issues.apache.org/jira/browse/LOG4...
GitHub user ssachin3108 opened a pull request:
https://github.com/apache/logging-log4net/pull/15
https://issues.apache.org/jira/browse/LOG4NET-552
services installed on production box are separate process and are configured to write log into single log file. Due to this following two issues are happening,
During write/append operation some processes are failing with error “Unable to acquire lock on file. The process cannot access the file because it is being used by another process”. This is because one process acquires lock (thread safe not process safe) for writing into log file and simultaneously another is trying to acquire lock and fails with above error and it just skips writing into log file.
During rolling operation, rolled file gets created with less than 1KB size. Thus log entries are lost. Upon investigation we found that, rolling operation is protected by system wide Mutex lock. At the time of rolling, multiple processes may come at the same time for rolling and first process will roll the original file correctly and give a different name to rolled file and re-create blank original file. Now the second process which would have come for rolling will roll the blank original file and overwrite the rolled file created by first process and thus rolled file is losing the data.
We locally have fixed above issues by changing latest log4net source code. We have kept locking model as “MinimalLock” which is current configuration in production. During append operation for acquirelock/releaselock we added system wide mutex so that each process will wait for other process to complete append operation. Thus it will not skip log from being written.
During rolling operation we added check whether rolling is already happened by some other process. If yes then skip rolling operation. This will resolve rolling file overwrite by other process issue.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ssachin3108/logging-log4net-1 develop
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/logging-log4net/pull/15.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #15
----
commit bd375a491e8477908b9085d71b6e309340a0e18d
Author: spatil <ss...@yahoo.com>
Date: 2017-08-18T08:46:30Z
https://issues.apache.org/jira/browse/LOG4NET-552
services installed on production box are separate process and are configured to write log into single log file. Due to this following two issues are happening,
During write/append operation some processes are failing with error “Unable to acquire lock on file. The process cannot access the file because it is being used by another process”. This is because one process acquires lock (thread safe not process safe) for writing into log file and simultaneously another is trying to acquire lock and fails with above error and it just skips writing into log file.
During rolling operation, rolled file gets created with less than 1KB size. Thus log entries are lost. Upon investigation we found that, rolling operation is protected by system wide Mutex lock. At the time of rolling, multiple processes may come at the same time for rolling and first process will roll the original file correctly and give a different name to rolled file and re-create blank original file. Now the second process which would have come for rolling will roll the blank original file and overwrite the rolled file created by first process and thus rolled file is losing the data.
We locally have fixed above issues by changing latest log4net source code. We have kept locking model as “MinimalLock” which is current configuration in production. During append operation for acquirelock/releaselock we added system wide mutex so that each process will wait for other process to complete append operation. Thus it will not skip log from being written.
During rolling operation we added check whether rolling is already happened by some other process. If yes then skip rolling operation. This will resolve rolling file overwrite by other process issue.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4net pull request #15: https://issues.apache.org/jira/browse/LOG4...
Posted by dpsenner <gi...@git.apache.org>.
Github user dpsenner commented on a diff in the pull request:
https://github.com/apache/logging-log4net/pull/15#discussion_r133924397
--- Diff: src/Appender/FileAppender.cs ---
@@ -620,22 +620,23 @@ public class MinimalLock : LockingModelBase
private string m_filename;
private bool m_append;
private Stream m_stream = null;
-
- /// <summary>
- /// Prepares to open the file when the first message is logged.
- /// </summary>
- /// <param name="filename">The filename to use</param>
- /// <param name="append">Whether to append to the file, or overwrite</param>
- /// <param name="encoding">The encoding to use</param>
- /// <remarks>
- /// <para>
- /// Open the file specified and prepare for logging.
- /// No writes will be made until <see cref="AcquireLock"/> is called.
- /// Must be called before any calls to <see cref="AcquireLock"/>,
- /// <see cref="ReleaseLock"/> and <see cref="CloseFile"/>.
- /// </para>
- /// </remarks>
- public override void OpenFile(string filename, bool append, Encoding encoding)
+ private Mutex m_appendMutex = null;
+ private string m_appendMutexFriendlyName = "MUTEX_INTERPROCESS_COMMUNICATION_ACROSS_PROCESSES";
--- End diff --
Should this be a more random string? `MUTEX_INTERPROCESS_COMMUNICATION_ACROSS_PROCESSES` might be in use by anyone. The mutex name should at least contain `log4net` and a scope what it is used for. Interprocess communication is not what the mutex locks and thus not the correct scope.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4net pull request #15: https://issues.apache.org/jira/browse/LOG4...
Posted by dpsenner <gi...@git.apache.org>.
Github user dpsenner commented on a diff in the pull request:
https://github.com/apache/logging-log4net/pull/15#discussion_r133924926
--- Diff: src/Appender/FileAppender.cs ---
@@ -620,22 +620,23 @@ public class MinimalLock : LockingModelBase
private string m_filename;
private bool m_append;
private Stream m_stream = null;
-
- /// <summary>
- /// Prepares to open the file when the first message is logged.
- /// </summary>
- /// <param name="filename">The filename to use</param>
- /// <param name="append">Whether to append to the file, or overwrite</param>
- /// <param name="encoding">The encoding to use</param>
- /// <remarks>
- /// <para>
- /// Open the file specified and prepare for logging.
- /// No writes will be made until <see cref="AcquireLock"/> is called.
- /// Must be called before any calls to <see cref="AcquireLock"/>,
- /// <see cref="ReleaseLock"/> and <see cref="CloseFile"/>.
- /// </para>
- /// </remarks>
- public override void OpenFile(string filename, bool append, Encoding encoding)
+ private Mutex m_appendMutex = null;
+ private string m_appendMutexFriendlyName = "MUTEX_INTERPROCESS_COMMUNICATION_ACROSS_PROCESSES";
+ /// <summary>
+ /// Prepares to open the file when the first message is logged.
+ /// </summary>
+ /// <param name="filename">The filename to use</param>
+ /// <param name="append">Whether to append to the file, or overwrite</param>
+ /// <param name="encoding">The encoding to use</param>
+ /// <remarks>
+ /// <para>
+ /// Open the file specified and prepare for logging.
+ /// No writes will be made until <see cref="AcquireLock"/> is called.
+ /// Must be called before any calls to <see cref="AcquireLock"/>,
+ /// <see cref="ReleaseLock"/> and <see cref="CloseFile"/>.
+ /// </para>
--- End diff --
Update the docstring to document the new behavior.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Antwort: [Newsletter] [GitHub] logging-log4net pull request #15:
https://issues.apache.org/jira/browse/LOG4...
Posted by Jo...@rohde-schwarz.com.
dpsenner <gi...@git.apache.org> wrote on 18.08.2017 12:05:29:
>
> Github user dpsenner commented on a diff in the pull request:
>
>
https://github.com/apache/logging-log4net/pull/15#discussion_r133923978
>
> --- Diff: src/Appender/FileAppender.cs ---
> @@ -695,7 +704,10 @@ public override void ReleaseLock()
> {
> CloseStream(m_stream);
> m_stream = null;
> - }
> +
> + if (m_appendMutex != null)
> + m_appendMutex.ReleaseMutex();
> --- End diff --
>
> Please wrap this in braces `{` and `}`.
When C#6 (nameof, et. al.) is acceptable, the one-liner
`m_appendMutex?.ReleaseMutex();` could be even nicer...
[GitHub] logging-log4net pull request #15: https://issues.apache.org/jira/browse/LOG4...
Posted by dpsenner <gi...@git.apache.org>.
Github user dpsenner commented on a diff in the pull request:
https://github.com/apache/logging-log4net/pull/15#discussion_r133923978
--- Diff: src/Appender/FileAppender.cs ---
@@ -695,7 +704,10 @@ public override void ReleaseLock()
{
CloseStream(m_stream);
m_stream = null;
- }
+
+ if (m_appendMutex != null)
+ m_appendMutex.ReleaseMutex();
--- End diff --
Please wrap this in braces `{` and `}`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4net pull request #15: https://issues.apache.org/jira/browse/LOG4...
Posted by ssachin3108 <gi...@git.apache.org>.
Github user ssachin3108 closed the pull request at:
https://github.com/apache/logging-log4net/pull/15
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4net pull request #15: https://issues.apache.org/jira/browse/LOG4...
Posted by dpsenner <gi...@git.apache.org>.
Github user dpsenner commented on a diff in the pull request:
https://github.com/apache/logging-log4net/pull/15#discussion_r133924767
--- Diff: src/Appender/FileAppender.cs ---
@@ -695,7 +704,10 @@ public override void ReleaseLock()
{
CloseStream(m_stream);
m_stream = null;
- }
+
+ if (m_appendMutex != null)
+ m_appendMutex.ReleaseMutex();
--- End diff --
Please wrap this in braces (`{` and `}`).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4net pull request #15: https://issues.apache.org/jira/browse/LOG4...
Posted by dpsenner <gi...@git.apache.org>.
Github user dpsenner commented on a diff in the pull request:
https://github.com/apache/logging-log4net/pull/15#discussion_r133923889
--- Diff: src/Appender/FileAppender.cs ---
@@ -671,7 +672,15 @@ public override Stream AcquireLock()
{
try
{
- m_stream = CreateStream(m_filename, m_append, FileShare.Read);
+ if (m_appendMutex == null)
+ {
+ if (!Mutex.TryOpenExisting(m_appendMutexFriendlyName, out m_appendMutex))
+ m_appendMutex = new Mutex(false, m_appendMutexFriendlyName);
--- End diff --
Please wrap this in braces (`{}`)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4net pull request #15: https://issues.apache.org/jira/browse/LOG4...
Posted by dpsenner <gi...@git.apache.org>.
Github user dpsenner commented on a diff in the pull request:
https://github.com/apache/logging-log4net/pull/15#discussion_r133924709
--- Diff: src/Appender/FileAppender.cs ---
@@ -671,7 +672,15 @@ public override Stream AcquireLock()
{
try
{
- m_stream = CreateStream(m_filename, m_append, FileShare.Read);
+ if (m_appendMutex == null)
+ {
+ if (!Mutex.TryOpenExisting(m_appendMutexFriendlyName, out m_appendMutex))
+ m_appendMutex = new Mutex(false, m_appendMutexFriendlyName);
--- End diff --
Please wrap this in braces (`{` and `}`)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4net pull request #15: https://issues.apache.org/jira/browse/LOG4...
Posted by dpsenner <gi...@git.apache.org>.
Github user dpsenner commented on a diff in the pull request:
https://github.com/apache/logging-log4net/pull/15#discussion_r133924628
--- Diff: src/Appender/FileAppender.cs ---
@@ -620,22 +620,23 @@ public class MinimalLock : LockingModelBase
private string m_filename;
private bool m_append;
private Stream m_stream = null;
-
- /// <summary>
- /// Prepares to open the file when the first message is logged.
- /// </summary>
- /// <param name="filename">The filename to use</param>
- /// <param name="append">Whether to append to the file, or overwrite</param>
- /// <param name="encoding">The encoding to use</param>
- /// <remarks>
- /// <para>
- /// Open the file specified and prepare for logging.
- /// No writes will be made until <see cref="AcquireLock"/> is called.
- /// Must be called before any calls to <see cref="AcquireLock"/>,
- /// <see cref="ReleaseLock"/> and <see cref="CloseFile"/>.
- /// </para>
- /// </remarks>
- public override void OpenFile(string filename, bool append, Encoding encoding)
+ private Mutex m_appendMutex = null;
+ private string m_appendMutexFriendlyName = "MUTEX_INTERPROCESS_COMMUNICATION_ACROSS_PROCESSES";
--- End diff --
Should this be a more random string? `MUTEX_INTERPROCESS_COMMUNICATION_ACROSS_PROCESSES` might be in use by anyone. The mutex name should at least contain `log4net` and a scope what it is used for. Interprocess communication is not what the mutex locks and thus not the correct scope.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---