[
https://issues.apache.org/jira/browse/GEODE-8806?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17262943#comment-17262943
]
ASF GitHub Bot commented on GEODE-8806:
---------------------------------------
pivotal-jbarrett commented on a change in pull request #718:
URL: https://github.com/apache/geode-native/pull/718#discussion_r555354528
##########
File path: cppcache/src/LocalRegion.cpp
##########
@@ -717,7 +717,7 @@ void LocalRegion::registerEntryExpiryTask(
new EntryExpiryHandler(rptr, entry, getEntryExpirationAction(),
duration);
auto id = rptr->getCacheImpl()->getExpiryTaskManager().scheduleExpiryTask(
handler, duration, std::chrono::seconds::zero());
- if (Log::finestEnabled()) {
+ if (Log::logLevel() >= LogLevel::Finest) {
Review comment:
I am sort of indifferent on this change but the original was more
closely aligned with the Java based logger API. This change requires you to
know that log levels have order.
##########
File path: clicache/src/Log.cpp
##########
@@ -70,12 +70,6 @@ namespace Apache
static_cast<apache::geode::client::LogLevel>(level));
}
- bool Log::Enabled(LogLevel level)
Review comment:
Doesn't this change the public API of the .NET library?
##########
File path: cppcache/src/util/Log.hpp
##########
@@ -182,315 +183,69 @@ class APACHE_GEODE_EXPORT Log {
* When invoking from outside either <init> should have been invoked,
* or at least the first invocation should be single-threaded.
*/
- static char* formatLogLine(char* buf, LogLevel level);
-
- /**
- * Returns whether log messages at given level are enabled.
- */
- static bool enabled(LogLevel level);
-
- /**
- * Logs a message at given level.
- */
- static void log(LogLevel level, const char* msg);
-
- /**
- * Logs both a message and thrown exception.
- */
- static void logThrow(LogLevel level, const char* msg, const Exception& ex);
-
- /**
- * Logs both a message and caught exception.
- */
- static void logCatch(LogLevel level, const char* msg, const Exception& ex);
-
- /**
- * Returns whether "error" log messages are enabled.
- */
- static bool errorEnabled();
-
- /**
- * Logs a message.
- * The message level is "error".
- */
- static void error(const char* msg);
-
- static void error(const std::string& msg);
-
- /**
- * Logs both a message and thrown exception.
- * The message level is "error".
- */
- static void errorThrow(const char* msg, const Exception& ex);
-
- /**
- * Writes both a message and caught exception.
- * The message level is "error".
- */
- static void errorCatch(const char* msg, const Exception& ex);
-
- /**
- * Returns whether "warning" log messages are enabled.
- */
- static bool warningEnabled();
-
- /**
- * Logs a message.
- * The message level is "warning".
- */
- static void warning(const char* msg);
-
- /**
- * Logs both a message and thrown exception.
- * The message level is "warning".
- */
- static void warningThrow(const char* msg, const Exception& ex);
-
- /**
- * Writes both a message and caught exception.
- * The message level is "warning".
- */
- static void warningCatch(const char* msg, const Exception& ex);
-
- /**
- * Returns whether "info" log messages are enabled.
- */
- static bool infoEnabled();
-
- /**
- * Logs a message.
- * The message level is "info".
- */
- static void info(const char* msg);
-
- /**
- * Logs both a message and thrown exception.
- * The message level is "info".
- */
- static void infoThrow(const char* msg, const Exception& ex);
-
- /**
- * Writes both a message and caught exception.
- * The message level is "info".
- */
- static void infoCatch(const char* msg, const Exception& ex);
-
- /**
- * Returns whether "config" log messages are enabled.
- */
- static bool configEnabled();
-
- /**
- * Logs a message.
- * The message level is "config".
- */
- static void config(const char* msg);
-
- /**
- * Logs both a message and thrown exception.
- * The message level is "config".
- */
- static void configThrow(const char* msg, const Exception& ex);
-
- /**
- * Writes both a message and caught exception.
- * The message level is "config".
- */
- static void configCatch(const char* msg, const Exception& ex);
-
- /**
- * Returns whether "fine" log messages are enabled.
- */
- static bool fineEnabled();
-
- /**
- * Logs a message.
- * The message level is "fine".
- */
- static void fine(const char* msg);
-
- /**
- * Logs both a message and thrown exception.
- * The message level is "fine".
- */
- static void fineThrow(const char* msg, const Exception& ex);
-
- /**
- * Writes both a message and caught exception.
- * The message level is "fine".
- */
- static void fineCatch(const char* msg, const Exception& ex);
-
- /**
- * Returns whether "finer" log messages are enabled.
- */
- static bool finerEnabled();
-
- /**
- * Logs a message.
- * The message level is "finer".
- */
- static void finer(const char* msg);
-
- /**
- * Logs both a message and thrown exception.
- * The message level is "finer".
- */
- static void finerThrow(const char* msg, const Exception& ex);
-
- /**
- * Writes both a message and caught exception.
- * The message level is "finer".
- */
- static void finerCatch(const char* msg, const Exception& ex);
-
- /**
- * Returns whether "finest" log messages are enabled.
- */
- static bool finestEnabled();
-
- /**
- * Logs a message.
- * The message level is "finest".
- */
- static void finest(const char* msg);
-
- /**
- * Logs both a message and thrown exception.
- * The message level is "finest".
- */
- static void finestThrow(const char* msg, const Exception& ex);
-
- /**
- * Writes both a message and caught exception.
- * The message level is "finest".
- */
- static void finestCatch(const char* msg, const Exception& ex);
-
- /**
- * Returns whether "debug" log messages are enabled.
- */
- static bool debugEnabled();
-
- /**
- * Logs a message.
- * The message level is "debug".
- */
- static void debug(const char* msg);
-
- /**
- * Logs both a message and thrown exception.
- * The message level is "debug".
- */
- static void debugThrow(const char* msg, const Exception& ex);
-
- /**
- * Writes both a message and caught exception.
- * The message level is "debug".
- */
- static void debugCatch(const char* msg, const Exception& ex);
-
- static void enterFn(LogLevel level, const char* functionName);
-
- static void exitFn(LogLevel level, const char* functionName);
+ static std::string formatLogLine(LogLevel level);
private:
static LogLevel s_logLevel;
static void writeBanner();
- public:
- static void put(LogLevel level, const std::string& msg);
-
- static void put(LogLevel level, const char* msg);
+ static void validateSizeLimits(int64_t fileSizeLimit, int64_t
diskSpaceLimit);
- static void putThrow(LogLevel level, const char* msg, const Exception& ex);
+ static void validateLogFileName(const std::string& filename);
- static void putCatch(LogLevel level, const char* msg, const Exception& ex);
-};
+ static void rollLogFile();
-class APACHE_GEODE_EXPORT LogFn {
- const char* m_functionName;
- LogLevel m_level;
+ static void removeOldestRolledLogFile();
- public:
- explicit LogFn(const char* functionName, LogLevel level = LogLevel::Finest);
+ static void buildRollFileMapping();
- ~LogFn();
+ static void setRollFileIndex();
- LogFn(const LogFn& rhs) = delete;
- LogFn& operator=(const LogFn& rhs) = delete;
-};
+ static void setSizeLimits(int32_t logFileLimit, int64_t logDiskSpaceLimit);
-class APACHE_GEODE_EXPORT LogVarargs {
public:
- static void debug(const char* fmt, ...);
- static void error(const char* fmt, ...);
- static void warn(const char* fmt, ...);
- static void info(const char* fmt, ...);
- static void config(const char* fmt, ...);
- static void fine(const char* fmt, ...);
- static void finer(const char* fmt, ...);
- static void finest(const char* fmt, ...);
+ static void log(LogLevel level, const std::string& msg);
+ static void log(LogLevel level, const char* fmt, ...);
- static void debug(const std::string& message);
+ private:
+ static void logInternal(LogLevel level, const std::string& msg);
+};
- static void error(const std::string& message);
+} // namespace client
+} // namespace geode
+} // namespace apache
- static void warn(const std::string& message);
+#define LOGERROR(...) \
+ ::apache::geode::client::Log::log(::apache::geode::client::LogLevel::Error, \
+ __VA_ARGS__)
- static void info(const std::string& message);
+#define LOGWARN(...) \
+ ::apache::geode::client::Log::log( \
+ ::apache::geode::client::LogLevel::Warning, __VA_ARGS__)
- static void config(const std::string& message);
+#define LOGINFO(...) \
+ ::apache::geode::client::Log::log(::apache::geode::client::LogLevel::Info, \
+ __VA_ARGS__)
- static void fine(const std::string& message);
+#define LOGCONFIG(...)
\
+ ::apache::geode::client::Log::log(::apache::geode::client::LogLevel::Config,
\
+ __VA_ARGS__)
- static void finer(const std::string& message);
+#define LOGFINE(...) \
+ ::apache::geode::client::Log::log(::apache::geode::client::LogLevel::Fine, \
+ __VA_ARGS__)
- static void finest(const std::string& message);
-};
-} // namespace client
-} // namespace geode
-} // namespace apache
+#define LOGFINER(...) \
+ ::apache::geode::client::Log::log(::apache::geode::client::LogLevel::Finer, \
+ __VA_ARGS__)
+
+#define LOGFINEST(...)
\
+ ::apache::geode::client::Log::log(::apache::geode::client::LogLevel::Finest,
\
+ __VA_ARGS__)
-#define LOGDEBUG \
- if (::apache::geode::client::LogLevel::Debug <= \
- ::apache::geode::client::Log::logLevel()) \
- ::apache::geode::client::LogVarargs::debug
-
-#define LOGERROR \
- if (::apache::geode::client::LogLevel::Error <= \
- ::apache::geode::client::Log::logLevel()) \
- ::apache::geode::client::LogVarargs::error
-
-#define LOGWARN \
- if (::apache::geode::client::LogLevel::Warning <= \
- ::apache::geode::client::Log::logLevel()) \
- ::apache::geode::client::LogVarargs::warn
-
-#define LOGINFO \
- if (::apache::geode::client::LogLevel::Info <= \
- ::apache::geode::client::Log::logLevel()) \
- ::apache::geode::client::LogVarargs::info
-
-#define LOGCONFIG \
- if (::apache::geode::client::LogLevel::Config <= \
- ::apache::geode::client::Log::logLevel()) \
- ::apache::geode::client::LogVarargs::config
-
-#define LOGFINE \
- if (::apache::geode::client::LogLevel::Fine <= \
- ::apache::geode::client::Log::logLevel()) \
- ::apache::geode::client::LogVarargs::fine
-
-#define LOGFINER \
- if (::apache::geode::client::LogLevel::Finer <= \
- ::apache::geode::client::Log::logLevel()) \
- ::apache::geode::client::LogVarargs::finer
-
-#define LOGFINEST \
- if (::apache::geode::client::LogLevel::Finest <= \
- ::apache::geode::client::Log::logLevel()) \
- ::apache::geode::client::LogVarargs::finest
+#define LOGDEBUG(...) \
Review comment:
This change results in the loss of the LogLevel check to avoid the cost
of executing functions that may make up the arguments to this log message. Now
all arguments will be resolved and the log message executed regardless of the
log message actually being loggable.
##########
File path: cppcache/src/statistics/GeodeStatisticsFactory.cpp
##########
@@ -61,8 +61,9 @@ GeodeStatisticsFactory::~GeodeStatisticsFactory() {
statsTypeMap.clear();
} catch (const Exception& ex) {
- Log::warningCatch("~GeodeStatisticsFactory swallowing Geode exception",
ex);
-
+ LOGWARN(std::string(
Review comment:
What is the rational for removing the a method that encapsulated the
deconstruction of an exception being logged?
----------------------------------------------------------------
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:
us...@infra.apache.org
> Native client does not honor log-file-size-limit properly
> ---------------------------------------------------------
>
> Key: GEODE-8806
> URL: https://issues.apache.org/jira/browse/GEODE-8806
> Project: Geode
> Issue Type: Bug
> Components: native client
> Reporter: Blake Bender
> Priority: Major
> Labels: pull-request-available
>
> As a user, I need to be able to limit the size of my log files and the amount
> of disk space used by geode-native. Unfortunately, the 10.x native client
> doesn't appear to "roll" log files correctly.
>
> repro steps:
> i. In a client application, set `log-file-size-limit` to 1 (for 1MB),
> `log-level` to `debug`, and `log-file` to something convenient like
> `native_app.log`
> ii. Run a long operation in the client that will generate a huge amount of
> logging, like 100,000 puts to a region
>
> expected behavior:
> * when native_app.log grows to > 1MB in size, it is followed by
> native_app-1.log, native_app-2.log, etc, each of which is ~1MB in size
> actual behavior:
> * native_app.log grows to > 1MB, then geode-native stops logging
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
