diff --git a/TODO.txt b/TODO.txt index 6bcc26dd24..34c620ba59 100644 --- a/TODO.txt +++ b/TODO.txt @@ -7,8 +7,15 @@ TODO - Add ICore interface - Make TxFormats a member of ICore instead of a singleton. +- Allow manual string labels for LogPartition, to fix the problem where + the log partition gets a file name like "ripple_LedgerConsensus" + - Rename LoadMonitor to LoadMeter, change LoadEvent to LoadMeter::ScopedSample +- Rewrite every beast Doxygen comment, update Beast Doxyfile + +- Rename ripple/beast include guards to boost style, e.g. RIPPLE_LOG_H_INCLUDED + - Fix all leaks on exit (!) Say there's a leak, a ledger that can never be accessed is locked in some @@ -56,6 +63,7 @@ TODO LoadEvent Is referenced with both a shared pointer and an auto pointer. + Should be named LoadMeter::ScopedSample JobQueue @@ -87,7 +95,9 @@ Verbose names Ledger "Skip List" - Is not really a skip list data structure + Is not really a skip list data structure. This is more appropriately + called an "index" although that name is currently used to identify hashes + used as keys. Duplicate Code @@ -111,14 +121,6 @@ Interfaces -------------------------------------------------------------------------------- -Implementation - - LoadManager - - What is going on in the destructor - --------------------------------------------------------------------------------- - boost Unclear from the class declaration what style of shared object management @@ -133,5 +135,5 @@ boost::recursive_mutex Recursive mutexes should never be necessary. They require the "mutable" keyword for const members to acquire the lock (yuck) - Replace recursive_mutex with juce::Mutex to remove boost dependency + Replace recursive_mutex with beast::Mutex to remove boost dependency diff --git a/modules/ripple_basics/containers/ripple_TaggedCache.cpp b/modules/ripple_basics/containers/ripple_TaggedCache.cpp index 989c0d7675..bd5bd4012a 100644 --- a/modules/ripple_basics/containers/ripple_TaggedCache.cpp +++ b/modules/ripple_basics/containers/ripple_TaggedCache.cpp @@ -4,4 +4,4 @@ */ //============================================================================== -SETUP_LOG (TaggedCacheLog) +SETUP_LOGN (TaggedCacheLog,"TaggedCache") diff --git a/modules/ripple_basics/utility/ripple_IniFile.cpp b/modules/ripple_basics/utility/ripple_IniFile.cpp index 6972fe096c..81b512f9e6 100644 --- a/modules/ripple_basics/utility/ripple_IniFile.cpp +++ b/modules/ripple_basics/utility/ripple_IniFile.cpp @@ -6,10 +6,9 @@ #define SECTION_DEFAULT_NAME "" -// for logging -struct ParseSectionLog { }; +struct ParseSectionLog; // for Log -SETUP_LOG (ParseSectionLog) +SETUP_LOGN (ParseSectionLog,"ParseSection") Section ParseSection (const std::string& strInput, const bool bTrim) { diff --git a/modules/ripple_basics/utility/ripple_Log.cpp b/modules/ripple_basics/utility/ripple_Log.cpp index b3c7a4df17..0e79bda066 100644 --- a/modules/ripple_basics/utility/ripple_Log.cpp +++ b/modules/ripple_basics/utility/ripple_Log.cpp @@ -12,16 +12,16 @@ std::ofstream* Log::outStream = NULL; boost::filesystem::path* Log::pathToLog = NULL; uint32 Log::logRotateCounter = 0; -#ifndef LOG_MAX_MESSAGE -#define LOG_MAX_MESSAGE (12 * 1024) -#endif +//------------------------------------------------------------------------------ LogPartition* LogPartition::headLog = NULL; -LogPartition::LogPartition (const char* name) : mNextLog (headLog), mMinSeverity (lsWARNING) +LogPartition::LogPartition (char const* partitionName) + : mNextLog (headLog) + , mMinSeverity (lsWARNING) { - const char* ptr = strrchr (name, '/'); - mName = (ptr == NULL) ? name : (ptr + 1); + const char* ptr = strrchr (partitionName, '/'); + mName = (ptr == NULL) ? partitionName : (ptr + 1); size_t p = mName.find (".cpp"); @@ -133,9 +133,9 @@ Log::~Log () logMsg += replaceFirstSecretWithAsterisks (oss.str ()); - if (logMsg.size () > LOG_MAX_MESSAGE) + if (logMsg.size () > maximumMessageCharacters) { - logMsg.resize (LOG_MAX_MESSAGE); + logMsg.resize (maximumMessageCharacters); logMsg += "..."; } diff --git a/modules/ripple_basics/utility/ripple_Log.h b/modules/ripple_basics/utility/ripple_Log.h index 6493a987b0..cb362c312d 100644 --- a/modules/ripple_basics/utility/ripple_Log.h +++ b/modules/ripple_basics/utility/ripple_Log.h @@ -4,8 +4,8 @@ */ //============================================================================== -#ifndef RIPPLE_LOG_H -#define RIPPLE_LOG_H +#ifndef RIPPLE_LOG_H_INCLUDED +#define RIPPLE_LOG_H_INCLUDED enum LogSeverity { @@ -20,23 +20,28 @@ enum LogSeverity //------------------------------------------------------------------------------ -// VFALCO TODO make this a nested class in Log -class LogPartition +// VFALCO TODO make this a nested class in Log? +class LogPartition // : public List ::Node { -protected: - static LogPartition* headLog; - - LogPartition* mNextLog; - LogSeverity mMinSeverity; - std::string mName; - public: - LogPartition (const char* name); + LogPartition (const char* partitionName); + + /** Retrieve the LogPartition associated with an object. + + Each LogPartition is a singleton. + */ + template + static LogPartition const& get () + { + static LogPartition logPartition (getPartitionName ()); + return logPartition; + } bool doLog (LogSeverity s) const { return s >= mMinSeverity; } + const std::string& getName () const { return mName; @@ -47,33 +52,34 @@ public: static std::vector< std::pair > getSeverities (); private: - /** Retrieve file name from a log partition. + /** Retrieve the name for a log partition. */ template - static char const* getFileName (); - /* - { - static_vfassert (false); - } - */ + static char const* getPartitionName (); -public: - template - static LogPartition const& get () - { - static LogPartition logPartition (getFileName ()); - return logPartition; - } +private: + // VFALCO TODO Use an intrusive linked list + // + static LogPartition* headLog; + + LogPartition* mNextLog; + LogSeverity mMinSeverity; + std::string mName; }; -#define SETUP_LOG(k) \ - template <> char const* LogPartition::getFileName () { return __FILE__; } \ - struct k##Instantiator { k##Instantiator () { LogPartition::get (); } }; \ - static k##Instantiator k##Instantiator_instance; +#define SETUP_LOG(Class) \ + template <> char const* LogPartition::getPartitionName () { return #Class; } \ + struct Class##Instantiator { Class##Instantiator () { LogPartition::get (); } }; \ + static Class##Instantiator Class##Instantiator_instance; + +#define SETUP_LOGN(Class,Name) \ + template <> char const* LogPartition::getPartitionName () { return Name; } \ + struct Class##Instantiator { Class##Instantiator () { LogPartition::get (); } }; \ + static Class##Instantiator Class##Instantiator_instance; //------------------------------------------------------------------------------ -class Log +class Log : public Uncopyable { public: explicit Log (LogSeverity s) : mSeverity (s) @@ -112,9 +118,15 @@ public: static std::string rotateLog (); private: - // VFALCO TODO derive from beast::Uncopyable - Log (const Log&); // no implementation - Log& operator= (const Log&); // no implementation + enum + { + /** Maximum line length for log messages. + + If the message exceeds this length it will be truncated + with elipses. + */ + maximumMessageCharacters = 12 * 1024 + }; // VFALCO TODO looks like there are really TWO classes in here. // One is a stream target for '<<' operator and the other diff --git a/src/cpp/ripple/LedgerTiming.cpp b/src/cpp/ripple/LedgerTiming.cpp index d9dd412054..8434d83c00 100644 --- a/src/cpp/ripple/LedgerTiming.cpp +++ b/src/cpp/ripple/LedgerTiming.cpp @@ -5,8 +5,10 @@ //============================================================================== // VFALCO Should rename ContinuousLedgerTiming to LedgerTiming -struct LedgerTimingLog; -SETUP_LOG (LedgerTimingLog) + +struct LedgerTiming; // for Log + +SETUP_LOG (LedgerTiming) // NOTE: First and last times must be repeated int ContinuousLedgerTiming::LedgerTimeResolution[] = { 10, 10, 20, 30, 60, 90, 120, 120 }; @@ -26,7 +28,7 @@ bool ContinuousLedgerTiming::shouldClose ( if ((previousMSeconds < -1000) || (previousMSeconds > 600000) || (currentMSeconds < -1000) || (currentMSeconds > 600000)) { - WriteLog (lsWARNING, LedgerTimingLog) << + WriteLog (lsWARNING, LedgerTiming) << boost::str (boost::format ("CLC::shouldClose range Trans=%s, Prop: %d/%d, Secs: %d (last:%d)") % (anyTransactions ? "yes" : "no") % previousProposers % proposersClosed % currentMSeconds % previousMSeconds); @@ -38,7 +40,7 @@ bool ContinuousLedgerTiming::shouldClose ( // no transactions so far this interval if (proposersClosed > (previousProposers / 4)) // did we miss a transaction? { - WriteLog (lsTRACE, LedgerTimingLog) << "no transactions, many proposers: now (" << proposersClosed << " closed, " + WriteLog (lsTRACE, LedgerTiming) << "no transactions, many proposers: now (" << proposersClosed << " closed, " << previousProposers << " before)"; return true; } @@ -47,7 +49,7 @@ bool ContinuousLedgerTiming::shouldClose ( if (previousMSeconds > (1000 * (LEDGER_IDLE_INTERVAL + 2))) // the last ledger was very slow to close { - WriteLog (lsTRACE, LedgerTimingLog) << "was slow to converge (p=" << (previousMSeconds) << ")"; + WriteLog (lsTRACE, LedgerTiming) << "was slow to converge (p=" << (previousMSeconds) << ")"; if (previousMSeconds < 2000) return previousMSeconds; @@ -61,13 +63,13 @@ bool ContinuousLedgerTiming::shouldClose ( if ((openMSeconds < LEDGER_MIN_CLOSE) && ((proposersClosed + proposersValidated) < (previousProposers / 2 ))) { - WriteLog (lsDEBUG, LedgerTimingLog) << "Must wait minimum time before closing"; + WriteLog (lsDEBUG, LedgerTiming) << "Must wait minimum time before closing"; return false; } if ((currentMSeconds < previousMSeconds) && ((proposersClosed + proposersValidated) < previousProposers)) { - WriteLog (lsDEBUG, LedgerTimingLog) << "We are waiting for more closes/validations"; + WriteLog (lsDEBUG, LedgerTiming) << "We are waiting for more closes/validations"; return false; } @@ -86,7 +88,7 @@ bool ContinuousLedgerTiming::haveConsensus ( bool forReal, // deciding whether to stop consensus process bool& failed) // we can't reach a consensus { - WriteLog (lsTRACE, LedgerTimingLog) << boost::str (boost::format ("CLC::haveConsensus: prop=%d/%d agree=%d validated=%d time=%d/%d%s") % + WriteLog (lsTRACE, LedgerTiming) << boost::str (boost::format ("CLC::haveConsensus: prop=%d/%d agree=%d validated=%d time=%d/%d%s") % currentProposers % previousProposers % currentAgree % currentFinished % currentAgreeTime % previousAgreeTime % (forReal ? "" : "X")); @@ -98,7 +100,7 @@ bool ContinuousLedgerTiming::haveConsensus ( // Less than 3/4 of the last ledger's proposers are present, we may need more time if (currentAgreeTime < (previousAgreeTime + LEDGER_MIN_CONSENSUS)) { - CondLog (forReal, lsTRACE, LedgerTimingLog) << "too fast, not enough proposers"; + CondLog (forReal, lsTRACE, LedgerTiming) << "too fast, not enough proposers"; return false; } } @@ -106,7 +108,7 @@ bool ContinuousLedgerTiming::haveConsensus ( // If 80% of current proposers (plus us) agree on a set, we have consensus if (((currentAgree * 100 + 100) / (currentProposers + 1)) > 80) { - CondLog (forReal, lsINFO, LedgerTimingLog) << "normal consensus"; + CondLog (forReal, lsINFO, LedgerTiming) << "normal consensus"; failed = false; return true; } @@ -114,13 +116,13 @@ bool ContinuousLedgerTiming::haveConsensus ( // If 80% of the nodes on your UNL have moved on, you should declare consensus if (((currentFinished * 100) / (currentProposers + 1)) > 80) { - CondLog (forReal, lsWARNING, LedgerTimingLog) << "We see no consensus, but 80% of nodes have moved on"; + CondLog (forReal, lsWARNING, LedgerTiming) << "We see no consensus, but 80% of nodes have moved on"; failed = true; return true; } // no consensus yet - CondLog (forReal, lsTRACE, LedgerTimingLog) << "no consensus"; + CondLog (forReal, lsTRACE, LedgerTiming) << "no consensus"; return false; } diff --git a/src/cpp/ripple/RPCErr.cpp b/src/cpp/ripple/RPCErr.cpp index 22fa0ab628..caa262fc7b 100644 --- a/src/cpp/ripple/RPCErr.cpp +++ b/src/cpp/ripple/RPCErr.cpp @@ -4,8 +4,7 @@ */ //============================================================================== -// For logging -struct RPCErr { }; +struct RPCErr; // for Log SETUP_LOG (RPCErr) diff --git a/src/cpp/ripple/WSConnection.cpp b/src/cpp/ripple/WSConnection.cpp index 75799eb9b1..7da13ba995 100644 --- a/src/cpp/ripple/WSConnection.cpp +++ b/src/cpp/ripple/WSConnection.cpp @@ -4,5 +4,4 @@ */ //============================================================================== - -SETUP_LOG (WSConnectionLog) +SETUP_LOGN (WSConnectionLog,"WSConnection") diff --git a/src/cpp/ripple/WSHandler.cpp b/src/cpp/ripple/WSHandler.cpp index 3b7d744cc9..c004050fac 100644 --- a/src/cpp/ripple/WSHandler.cpp +++ b/src/cpp/ripple/WSHandler.cpp @@ -4,5 +4,4 @@ */ //============================================================================== - -SETUP_LOG (WSServerHandlerLog) +SETUP_LOGN (WSServerHandlerLog,"WSServerHandler") diff --git a/src/cpp/ripple/ripple_SerializedLedger.cpp b/src/cpp/ripple/ripple_SerializedLedger.cpp index 1a6f3012c6..3a19036c6d 100644 --- a/src/cpp/ripple/ripple_SerializedLedger.cpp +++ b/src/cpp/ripple/ripple_SerializedLedger.cpp @@ -4,10 +4,9 @@ */ //============================================================================== -// For logging -struct SerializedLedgerLog; +struct SerializedLedgerLog; // for Log -SETUP_LOG (SerializedLedgerLog) +SETUP_LOGN (SerializedLedgerLog,"SerializedLedger") SerializedLedgerEntry::SerializedLedgerEntry (SerializerIterator& sit, uint256 const& index) : STObject (sfLedgerEntry), mIndex (index), mMutable (true)