You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by sw...@apache.org on 2026/10/16 23:47:33 UTC
(logging-log4cxx) 01/01: Simplify MultiprocessRollingFileAppender
This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch simplify_multiprocess_fileappender
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit 3ad5be539b3d6b14ae45716fbcad78650f1ac900
Author: Stephen Webb <sw...@gmail.com>
AuthorDate: Sat Oct 17 10:47:09 2026 +1100
Simplify MultiprocessRollingFileAppender
---
src/main/cpp/multiprocessrollingfileappender.cpp | 227 +++++++--------------
src/main/cpp/rollingfileappender.cpp | 4 -
.../log4cxx/private/rollingfileappender_priv.h | 5 -
.../rolling/multiprocessrollingfileappender.h | 53 +----
4 files changed, 75 insertions(+), 214 deletions(-)
diff --git a/src/main/cpp/multiprocessrollingfileappender.cpp b/src/main/cpp/multiprocessrollingfileappender.cpp
index 00ad6599..9c329024 100644
--- a/src/main/cpp/multiprocessrollingfileappender.cpp
+++ b/src/main/cpp/multiprocessrollingfileappender.cpp
@@ -51,14 +51,49 @@ using namespace LOG4CXX_NS::spi;
struct MultiprocessRollingFileAppender::MultiprocessRollingFileAppenderPriv
: public RollingFileAppenderPriv
{
+ FileOutputStreamPtr fileOutputStream;
+ apr_file_t* lock_file = NULL;
+
~MultiprocessRollingFileAppenderPriv()
{
if (lock_file)
apr_file_close(lock_file);
}
-
- FileOutputStreamPtr fileOutputStream;
- apr_file_t* lock_file = NULL;
+ /**
+ * Coordinate a rollover with other processes
+
+ * @return true if this process perfomed the rollover.
+ */
+ bool synchronizedRollover
+ ( Appender* appender
+ , const spi::LoggingEventPtr& event
+ , const LogString& fileName
+ , bool append
+ , const TriggeringPolicyPtr& trigger
+ );
+
+ /**
+ * Is it possible the current log file was renamed?
+ */
+ bool isRolloverCheckNeeded();
+
+ /**
+ * Was \c fileName renamed?
+ * @param pSize if not NULL, receives the log file size
+ * @return true if the log file must be reopened
+ */
+ bool isAlreadyRolled(const LogString& fileName, size_t* pSize = 0);
+
+ /**
+ * Put the current size of the log file into \c pSize.
+ * @return true if the log file size was put into \c pSize
+ */
+ bool getCurrentFileSize(size_t* pSize);
+
+ /**
+ * re-open \c fileName (used after it has been renamed)
+ */
+ void reopenFile(const LogString& fileName);
public: // Support classes
class Lock
@@ -169,11 +204,11 @@ void MultiprocessRollingFileAppender::activateOptions( )
/**
* Is it possible the current log file was renamed?
*/
-bool MultiprocessRollingFileAppender::isRolloverCheckNeeded()
+bool MultiprocessRollingFileAppender::MultiprocessRollingFileAppenderPriv::isRolloverCheckNeeded()
{
bool result = true;
#if USING_ROLLOVER_REQUIRED_CHECK_IS_FASTER
- if (auto pTimeBased = LOG4CXX_NS::cast<TimeBasedRollingPolicy>(_priv->rollingPolicy))
+ if (auto pTimeBased = LOG4CXX_NS::cast<TimeBasedRollingPolicy>(this->rollingPolicy))
result = !pTimeBased->isLastFileNameUnchanged();
#endif
return result;
@@ -182,15 +217,15 @@ bool MultiprocessRollingFileAppender::isRolloverCheckNeeded()
/**
* Was \c fileName renamed?
*/
-bool MultiprocessRollingFileAppender::isAlreadyRolled(const LogString& fileName, size_t* pSize)
+bool MultiprocessRollingFileAppender::MultiprocessRollingFileAppenderPriv::isAlreadyRolled(const LogString& fileName, size_t* pSize)
{
- if( !_priv->fileOutputStream || !_priv->fileOutputStream->getFilePtr() )
+ if( !this->fileOutputStream || !this->fileOutputStream->getFilePtr() )
return false;
apr_int32_t wantedInfo = APR_FINFO_IDENT;
if (pSize)
wantedInfo |= APR_FINFO_SIZE;
apr_finfo_t finfo1;
- apr_status_t st1 = apr_file_info_get(&finfo1, wantedInfo, _priv->fileOutputStream->getFilePtr());
+ apr_status_t st1 = apr_file_info_get(&finfo1, wantedInfo, this->fileOutputStream->getFilePtr());
if (st1 != APR_SUCCESS)
LogLog::warn(LOG4CXX_STR("apr_file_info_get failed"));
@@ -201,7 +236,7 @@ bool MultiprocessRollingFileAppender::isAlreadyRolled(const LogString& fileName,
apr_status_t st2;
apr_finfo_t finfo2;
int retryCount = 0;
- while (APR_SUCCESS != (st2 = apr_stat(&finfo2, fname.c_str(), wantedInfo, _priv->pool.getAPRPool())))
+ while (APR_SUCCESS != (st2 = apr_stat(&finfo2, fname.c_str(), wantedInfo, this->pool.getAPRPool())))
{
if (5 == ++retryCount)
break;
@@ -221,13 +256,13 @@ bool MultiprocessRollingFileAppender::isAlreadyRolled(const LogString& fileName,
/**
* Put the current size of the log file into \c pSize.
*/
-bool MultiprocessRollingFileAppender::getCurrentFileSize(size_t* pSize)
+bool MultiprocessRollingFileAppender::MultiprocessRollingFileAppenderPriv::getCurrentFileSize(size_t* pSize)
{
- if( !_priv->fileOutputStream || !_priv->fileOutputStream->getFilePtr() )
+ if( !this->fileOutputStream || !this->fileOutputStream->getFilePtr() )
return false;
apr_int32_t wantedInfo = APR_FINFO_SIZE;
apr_finfo_t finfo;
- if (apr_file_info_get(&finfo, wantedInfo, _priv->fileOutputStream->getFilePtr()) != APR_SUCCESS)
+ if (apr_file_info_get(&finfo, wantedInfo, this->fileOutputStream->getFilePtr()) != APR_SUCCESS)
{
LogLog::warn(LOG4CXX_STR("apr_file_info_get failed"));
return false;
@@ -236,137 +271,33 @@ bool MultiprocessRollingFileAppender::getCurrentFileSize(size_t* pSize)
return true;
}
-/**
- Implements the usual roll over behaviour.
-
- <p>If <code>MaxBackupIndex</code> is positive, then files
- {<code>File.1</code>, ..., <code>File.MaxBackupIndex -1</code>}
- are renamed to {<code>File.2</code>, ...,
- <code>File.MaxBackupIndex</code>}. Moreover, <code>File</code> is
- renamed <code>File.1</code> and closed. A new <code>File</code> is
- created to receive further log output.
-
- <p>If <code>MaxBackupIndex</code> is equal to zero, then the
- <code>File</code> is truncated with no backup files created.
-
- * @return true if rollover performed.
- */
-bool MultiprocessRollingFileAppender::rollover()
-{
- std::lock_guard<std::recursive_mutex> lock(_priv->mutex);
- return synchronizedRollover();
-}
-
/**
* Coordinate a rollover with other processes
* @return true if this process perfomed the rollover.
*/
-bool MultiprocessRollingFileAppender::synchronizedRollover(const TriggeringPolicyPtr& trigger)
+bool MultiprocessRollingFileAppender::MultiprocessRollingFileAppenderPriv::synchronizedRollover
+ ( Appender* appender
+ , const spi::LoggingEventPtr& event
+ , const LogString& fileName
+ , bool append
+ , const TriggeringPolicyPtr& trigger
+ )
{
bool result = false;
- LogString fileName = getFile();
- if (!_priv->rollingPolicy)
+ if (!this->rollingPolicy)
; // can't roll without a policy
- else if (isAlreadyRolled(fileName, &_priv->fileLength))
- reopenFile(fileName);
+ else if (this->isAlreadyRolled(fileName, &this->fileLength))
+ this->reopenFile(fileName);
else
{
- helpers::Pool tempPool;
- MultiprocessRollingFileAppenderPriv::Lock lk(_priv, fileName);
+ MultiprocessRollingFileAppenderPriv::Lock lk(this, fileName);
if (!lk.hasLock())
LogLog::warn(LOG4CXX_STR("Failed to lock ") + fileName);
- else if (isAlreadyRolled(fileName, &_priv->fileLength))
- reopenFile(fileName);
- else if (trigger && !trigger->isTriggeringEvent(this, _priv->event, fileName, _priv->fileLength))
- ;
- else if (auto rollover1 = _priv->rollingPolicy->rollover(fileName, getAppend()))
- {
- _priv->close();
- if (rollover1->getActiveFileName() == fileName)
- {
- bool success = true; // A synchronous action is not required
- if (auto pAction = rollover1->getSynchronous())
- success = pAction->execute();
-
- bool appendToExisting = true;
- if (success)
- {
- appendToExisting = rollover1->getAppend();
- if (appendToExisting)
- {
- _priv->fileLength = File().setPath(rollover1->getActiveFileName()).length();
- }
- else
- {
- _priv->fileLength = 0;
- }
-
- if (auto asyncAction = rollover1->getAsynchronous())
- {
- try
- {
- asyncAction->execute();
- }
- catch (std::exception& ex)
- {
- LogString msg(LOG4CXX_STR("Async action in rollover ["));
- msg.append(fileName);
- msg.append(LOG4CXX_STR("] failed"));
- _priv->errorHandler->error(msg, ex, 0);
- }
- }
- }
- else
- {
- LogString msg(LOG4CXX_STR("Rollover of ["));
- msg.append(fileName);
- msg.append(LOG4CXX_STR("] failed"));
- _priv->errorHandler->error(msg);
- }
- _priv->targetFile.setPath(rollover1->getActiveFileName());
- _priv->fileAppend = appendToExisting;
- _priv->activate();
- }
- else
- {
- _priv->targetFile.setPath(rollover1->getActiveFileName());
- _priv->fileAppend = rollover1->getAppend();
- _priv->activate();
- bool success = true; // A synchronous action is not required
- if (auto pAction = rollover1->getSynchronous())
- success = pAction->execute();
-
- if (success)
- {
- if (rollover1->getAppend())
- {
- _priv->fileLength = File().setPath(rollover1->getActiveFileName()).length();
- }
- else
- {
- _priv->fileLength = 0;
- }
-
- if (auto asyncAction = rollover1->getAsynchronous())
- {
- try
- {
- asyncAction->execute();
- }
- catch (std::exception& ex)
- {
- LogString msg(LOG4CXX_STR("Async action in rollover ["));
- msg.append(fileName);
- msg.append(LOG4CXX_STR("] failed"));
- _priv->errorHandler->error(msg, ex, 0);
- }
- }
- }
- }
-
- result = true;
- }
+ else if (this->isAlreadyRolled(fileName, &this->fileLength))
+ this->reopenFile(fileName);
+ else if (!trigger || trigger->isTriggeringEvent(appender, event, fileName, this->fileLength))
+ result = this->rollover(fileName, append);
}
return result;
@@ -375,27 +306,23 @@ bool MultiprocessRollingFileAppender::synchronizedRollover(const TriggeringPolic
/**
* re-open \c fileName (used after it has been renamed)
*/
-void MultiprocessRollingFileAppender::reopenFile(const LogString& fileName)
+void MultiprocessRollingFileAppender::MultiprocessRollingFileAppenderPriv::reopenFile(const LogString& fileName)
{
- _priv->close();
+ this->close();
#if USING_ROLLOVER_REQUIRED_CHECK_IS_FASTER
- if (auto pTimeBased = LOG4CXX_NS::cast<TimeBasedRollingPolicy>(_priv->rollingPolicy))
+ if (auto pTimeBased = LOG4CXX_NS::cast<TimeBasedRollingPolicy>(this->rollingPolicy))
pTimeBased->loadLastFileName();
#endif
- OutputStreamPtr os = std::make_shared<FileOutputStream>(fileName, true, _priv->lazyFileCreation);
- WriterPtr newWriter(_priv->createWriter(os));
- setFile(fileName);
- _priv->setWriter(newWriter);
+ this->targetFile.setPath(fileName);
+ auto os = std::make_shared<FileOutputStream>(fileName, true, this->lazyFileCreation);
+ this->setWriter(this->createWriter(os));
}
-/**
- * {@inheritDoc}
-*/
void MultiprocessRollingFileAppender::subAppend( const spi::LoggingEventPtr& event )
{
// The rollover check must precede actual writing. This is the
// only correct behavior for time driven triggers.
- LogString fileName = getFile();
+ auto fileName = getFile();
if (_priv->triggeringPolicy->isTriggeringEvent(this, event, fileName, _priv->fileLength))
{
//
@@ -405,8 +332,7 @@ void MultiprocessRollingFileAppender::subAppend( const spi::LoggingEventPtr& eve
// condition and the append should still happen.
try
{
- _priv->event = event;
- synchronizedRollover(_priv->triggeringPolicy);
+ _priv->synchronizedRollover(this, event, fileName, getAppend(), _priv->triggeringPolicy);
}
catch (std::exception& ex)
{
@@ -416,10 +342,10 @@ void MultiprocessRollingFileAppender::subAppend( const spi::LoggingEventPtr& eve
_priv->errorHandler->error(msg, ex, 0);
}
}
- else if (!isRolloverCheckNeeded())
- getCurrentFileSize(&_priv->fileLength);
- else if (isAlreadyRolled(fileName, &_priv->fileLength))
- reopenFile(fileName);
+ else if (!_priv->isRolloverCheckNeeded())
+ _priv->getCurrentFileSize(&_priv->fileLength);
+ else if (_priv->isAlreadyRolled(fileName, &_priv->fileLength))
+ _priv->reopenFile(fileName);
FileAppender::subAppend( event );
}
@@ -438,8 +364,3 @@ WriterPtr MultiprocessRollingFileAppender::MultiprocessRollingFileAppenderPriv::
return RollingFileAppenderPriv::createWriter(os);
}
-
-void MultiprocessRollingFileAppender::setFileLength(size_t length)
-{
- _priv->fileLength = length;
-}
diff --git a/src/main/cpp/rollingfileappender.cpp b/src/main/cpp/rollingfileappender.cpp
index 8aac45ec..6798ea12 100644
--- a/src/main/cpp/rollingfileappender.cpp
+++ b/src/main/cpp/rollingfileappender.cpp
@@ -398,9 +398,6 @@ bool RollingFileAppender::RollingFileAppenderPriv::nameEvolvingRollover
return result;
}
-/**
- * {@inheritDoc}
-*/
void RollingFileAppender::subAppend( const spi::LoggingEventPtr& event )
{
// The rollover check must precede actual writing. This is the
@@ -416,7 +413,6 @@ void RollingFileAppender::subAppend( const spi::LoggingEventPtr& event )
// condition and the append should still happen.
try
{
- _priv->event = event;
_priv->rollover(getFile(), getAppend());
}
catch (std::exception& ex)
diff --git a/src/main/include/log4cxx/private/rollingfileappender_priv.h b/src/main/include/log4cxx/private/rollingfileappender_priv.h
index c04bea6b..a8e1fdbb 100644
--- a/src/main/include/log4cxx/private/rollingfileappender_priv.h
+++ b/src/main/include/log4cxx/private/rollingfileappender_priv.h
@@ -47,11 +47,6 @@ struct RollingFileAppender::RollingFileAppenderPriv : public FileAppenderPriv
*/
size_t fileLength;
- /**
- * save the loggingevent
- */
- spi::LoggingEventPtr event;
-
bool activateOptions();
helpers::WriterPtr createWriter(const helpers::OutputStreamPtr& os) override;
diff --git a/src/main/include/log4cxx/rolling/multiprocessrollingfileappender.h b/src/main/include/log4cxx/rolling/multiprocessrollingfileappender.h
index c0bd6bf5..2739d7b9 100644
--- a/src/main/include/log4cxx/rolling/multiprocessrollingfileappender.h
+++ b/src/main/include/log4cxx/rolling/multiprocessrollingfileappender.h
@@ -62,64 +62,13 @@ class LOG4CXX_EXPORT MultiprocessRollingFileAppender : public RollingFileAppende
*/
void activateOptions( ) override;
- /**
- Implements the usual roll over behaviour.
-
- <p>If <code>MaxBackupIndex</code> is positive, then files
- {<code>File.1</code>, ..., <code>File.MaxBackupIndex -1</code>}
- are renamed to {<code>File.2</code>, ...,
- <code>File.MaxBackupIndex</code>}. Moreover, <code>File</code> is
- renamed <code>File.1</code> and closed. A new <code>File</code> is
- created to receive further log output.
-
- <p>If <code>MaxBackupIndex</code> is equal to zero, then the
- <code>File</code> is truncated with no backup files created.
-
- */
- bool rollover();
-
protected:
/**
- Actual writing occurs here.
+ Rollover the file if required before calling WriterAppender::subAppend.
*/
void subAppend( const spi::LoggingEventPtr& event ) override;
- private:
- /**
- * Coordinate a rollover with other processes
-
- * @return true if this process perfomed the rollover.
- */
- bool synchronizedRollover(const TriggeringPolicyPtr& trigger = TriggeringPolicyPtr() );
-
- /**
- * Set the length of current active log file to \c length bytes.
- */
- void setFileLength(size_t length);
-
- /**
- * Is it possible the current log file was renamed?
- */
- bool isRolloverCheckNeeded();
-
- /**
- * Was \c fileName renamed?
- * @param pSize if not NULL, receives the log file size
- * @return true if the log file must be reopened
- */
- bool isAlreadyRolled(const LogString& fileName, size_t* pSize = 0);
-
- /**
- * Put the current size of the log file into \c pSize.
- * @return true if the log file size was put into \c pSize
- */
- bool getCurrentFileSize(size_t* pSize);
-
- /**
- * re-open \c fileName (used after it has been renamed)
- */
- void reopenFile(const LogString& fileName);
};
LOG4CXX_PTR_DEF(MultiprocessRollingFileAppender);