Clean up and plug leak in snapshot thread SERVER-768

4 files changed, 39 insertions, 41 deletions

diff --git a/db/stats/snapshots.cpp b/db/stats/snapshots.cpp
index 7fc6bbdc0da..3ac2a1fa629 100644
--- a/db/stats/snapshots.cpp
+++ b/db/stats/snapshots.cpp
@@ -25,50 +25,49 @@
    handles snapshotting performance metrics and other such things
  */
 namespace mongo {
-
-    SnapshotData::SnapshotData()
-        : _created ( curTimeMicros64() ) , 
-          _globalUsage( Top::global.getGlobalData() )
-    {
+    void SnapshotData::takeSnapshot(){
+         _created = curTimeMicros64();
+         _globalUsage = Top::global.getGlobalData();
         _totalWriteLockedTime = dbMutex.info().getTimeLocked();
         _usage = Top::global.cloneMap();
     }
 
-    SnapshotDelta::SnapshotDelta( SnapshotData * older , SnapshotData * newer )
+    SnapshotDelta::SnapshotDelta( const SnapshotData& older , const SnapshotData& newer )
         : _older( older ) , _newer( newer )
     {
-        assert( _newer->_created > _older->_created );
-        _elapsed = _newer->_created - _older->_created;
+        assert( _newer._created > _older._created );
+        _elapsed = _newer._created - _older._created;
         
     }
     
     Top::CollectionData SnapshotDelta::globalUsageDiff(){
-        return Top::CollectionData( _older->_globalUsage , _newer->_globalUsage );
+        return Top::CollectionData( _older._globalUsage , _newer._globalUsage );
     }
     Top::UsageMap SnapshotDelta::collectionUsageDiff(){
         Top::UsageMap u;
         
-        for ( Top::UsageMap::iterator i=_newer->_usage.begin(); i != _newer->_usage.end(); i++ ){
-            u[i->first] = Top::CollectionData( _older->_usage[i->first] , i->second );
+        for ( Top::UsageMap::const_iterator i=_newer._usage.begin(); i != _newer._usage.end(); i++ ){
+            Top::UsageMap::const_iterator j = _older._usage.find(i->first);
+            if (j != _older._usage.end())
+                u[i->first] = Top::CollectionData( j->second , i->second );
         }
         return u;
     }
 
-    Snapshots::Snapshots(){
-        _n = 100;
-        _snapshots = new SnapshotData*[_n];
-        for ( int i=0; i<_n; i++ )
-            _snapshots[i] = 0;
-        _loc = 0;
-        _stored = 0;
-    }
+    Snapshots::Snapshots(int n)
+        : _n(n)
+        , _snapshots(new SnapshotData[n])
+        , _loc(0)
+        , _stored(0)
+    {}
     
-    void Snapshots::add( SnapshotData * s ){
+    const SnapshotData* Snapshots::takeSnapshot(){
         scoped_lock lk(_lock);
         _loc = ( _loc + 1 ) % _n;
-        _snapshots[_loc] = s;
+        _snapshots[_loc].takeSnapshot();
         if ( _stored < _n )
             _stored++;
+        return &_snapshots[_loc];
     }
 
     auto_ptr<SnapshotDelta> Snapshots::computeDelta( int numBack ){
@@ -79,7 +78,7 @@ namespace mongo {
         return p;
     }
 
-    SnapshotData* Snapshots::getPrev( int numBack ){
+    const SnapshotData& Snapshots::getPrev( int numBack ){
         int x = _loc - numBack;
         if ( x < 0 )
             x += _n;
@@ -109,19 +108,17 @@ namespace mongo {
 
         long long numLoops = 0;
         
-        SnapshotData * prev = 0;
+        const SnapshotData* prev = 0;
 
         while ( ! inShutdown() ){
             try {
-                SnapshotData * s = new SnapshotData();
-                
-                statsSnapshots.add( s );
+                const SnapshotData* s = statsSnapshots.takeSnapshot();
                 
                 if ( prev ){
                     unsigned long long elapsed = s->_created - prev->_created;
 
                     if ( cmdLine.cpu ){
-                        SnapshotDelta d( prev , s );
+                        SnapshotDelta d( *prev , *s );
                         log() << "cpu: elapsed:" << (elapsed/1000) <<"  writelock: " << (int)(100*d.percentWriteLocked()) << "%" << endl;
                     }
 
diff --git a/db/stats/snapshots.h b/db/stats/snapshots.h
index 9fde41ac1ee..542318aaee6 100644
--- a/db/stats/snapshots.h
+++ b/db/stats/snapshots.h
@@ -34,7 +34,7 @@ namespace mongo {
      * i.e. all counters at a given time
      */
     class SnapshotData {
-        SnapshotData();
+        void takeSnapshot();
 
         unsigned long long _created;
         Top::CollectionData _globalUsage;
@@ -43,6 +43,7 @@ namespace mongo {
 
         friend class SnapshotThread;
         friend class SnapshotDelta;
+        friend class Snapshots;
     };
     
     /**
@@ -50,10 +51,10 @@ namespace mongo {
      */
     class SnapshotDelta {
     public:
-        SnapshotDelta( SnapshotData * older , SnapshotData * newer );
+        SnapshotDelta( const SnapshotData& older , const SnapshotData& newer );
         
         unsigned long long start() const {
-            return _older->_created;
+            return _older._created;
         }
 
         unsigned long long elapsed() const {
@@ -61,7 +62,7 @@ namespace mongo {
         }
         
         unsigned long long timeInWriteLock() const {
-            return _newer->_totalWriteLockedTime - _older->_totalWriteLockedTime;
+            return _newer._totalWriteLockedTime - _older._totalWriteLockedTime;
         }
         double percentWriteLocked() const {
             double e = (double) elapsed();
@@ -73,21 +74,21 @@ namespace mongo {
         Top::UsageMap collectionUsageDiff();
 
     private:
-        SnapshotData * _older;
-        SnapshotData * _newer;
+        const SnapshotData& _older;
+        const SnapshotData& _newer;
 
         unsigned long long _elapsed;
     };
 
     class Snapshots {
     public:
-        Snapshots();
+        Snapshots(int n=100);
         
-        void add( SnapshotData * s );
+        const SnapshotData* takeSnapshot();
         
         int numDeltas() const { return _stored-1; }
 
-        SnapshotData* getPrev( int numBack = 0 );
+        const SnapshotData& getPrev( int numBack = 0 );
         auto_ptr<SnapshotDelta> computeDelta( int numBack = 0 );
         
         
@@ -95,7 +96,7 @@ namespace mongo {
     private:
         mongo::mutex _lock;
         int _n;
-        SnapshotData** _snapshots;
+        boost::scoped_array<SnapshotData> _snapshots;
         int _loc;
         int _stored;
     };
diff --git a/db/stats/top.cpp b/db/stats/top.cpp
index 93d29ed3baf..1d4f670db95 100644
--- a/db/stats/top.cpp
+++ b/db/stats/top.cpp
@@ -23,14 +23,14 @@
 
 namespace mongo {
     
-    Top::UsageData::UsageData( UsageData& older , UsageData& newer )
+    Top::UsageData::UsageData( const UsageData& older , const UsageData& newer )
         : time(newer.time-older.time) , 
           count(newer.count-older.count) 
     {
         
     }
 
-    Top::CollectionData::CollectionData( CollectionData& older , CollectionData& newer )
+    Top::CollectionData::CollectionData( const CollectionData& older , const CollectionData& newer )
         : total( older.total , newer.total ) , 
           readLock( older.readLock , newer.readLock ) ,
           writeLock( older.writeLock , newer.writeLock ) ,
diff --git a/db/stats/top.h b/db/stats/top.h
index eaf8a12cad5..66240aeef42 100644
--- a/db/stats/top.h
+++ b/db/stats/top.h
@@ -32,7 +32,7 @@ namespace mongo {
         class UsageData {
         public:
             UsageData() : time(0) , count(0){}
-            UsageData( UsageData& older , UsageData& newer );
+            UsageData( const UsageData& older , const UsageData& newer );
             long long time;
             long long count;
 
@@ -48,7 +48,7 @@ namespace mongo {
              * constructs a diff
              */
             CollectionData(){}
-            CollectionData( CollectionData& older , CollectionData& newer );
+            CollectionData( const CollectionData& older , const CollectionData& newer );
             
             UsageData total;