Refactor Application shutdown using new Service, AsyncService interfaces

src/ripple_core/functional/Job.cpp

@@ -16,12 +16,24 @@ Job::Job (JobType type, uint64 index)
 {
 }
 
+Job::Job (Job const& other)
+    : m_cancelCallback (other.m_cancelCallback)
+    , mType (other.mType)
+    , mJobIndex (other.mJobIndex)
+    , mJob (other.mJob)
+    , m_loadEvent (other.m_loadEvent)
+    , mName (other.mName)
+{
+}
+
 Job::Job (JobType type,
           std::string const& name,
           uint64 index,
           LoadMonitor& lm,
-          FUNCTION_TYPE <void (Job&)> const& job)
-    : mType (type)
+          FUNCTION_TYPE <void (Job&)> const& job,
+          CancelCallback cancelCallback)
+    : m_cancelCallback (cancelCallback)
+    , mType (type)
     , mJobIndex (index)
     , mJob (job)
     , mName (name)
@@ -29,11 +41,35 @@ Job::Job (JobType type,
     m_loadEvent = boost::make_shared <LoadEvent> (boost::ref (lm), name, false);
 }
 
+Job& Job::operator= (Job const& other)
+{
+    mType = other.mType;
+    mJobIndex = other.mJobIndex;
+    mJob = other.mJob;
+    m_loadEvent = other.m_loadEvent;
+    mName = other.mName;
+    m_cancelCallback = other.m_cancelCallback;
+    return *this;
+}
+
 JobType Job::getType () const
 {
     return mType;
 }
 
+CancelCallback Job::getCancelCallback () const
+{
+    bassert (! m_cancelCallback.empty());
+    return m_cancelCallback;
+}
+
+bool Job::shouldCancel () const
+{
+    if (! m_cancelCallback.empty ())
+        return m_cancelCallback ();
+    return false;
+}
+
 void Job::doJob ()
 {
     m_loadEvent->reName (mName);

src/ripple_core/functional/Job.h

@@ -67,6 +67,8 @@ public:
     //
     Job ();
 
+    Job (Job const& other);
+
     Job (JobType type, uint64 index);
 
     // VFALCO TODO try to remove the dependency on LoadMonitor.
@@ -74,10 +76,18 @@ public:
          std::string const& name,
          uint64 index,
          LoadMonitor& lm,
-         FUNCTION_TYPE <void (Job&)> const& job);
+         FUNCTION_TYPE <void (Job&)> const& job,
+         CancelCallback cancelCallback);
+
+    Job& operator= (Job const& other);
 
     JobType getType () const;
 
+    CancelCallback getCancelCallback () const;
+
+    /** Returns `true` if the running job should make a best-effort cancel. */
+    bool shouldCancel () const;
+
     void doJob ();
 
     void rename (const std::string& n);
@@ -93,6 +103,7 @@ public:
     static const char* toString (JobType);
 
 private:
+    CancelCallback m_cancelCallback;
     JobType                     mType;
     uint64                      mJobIndex;
     FUNCTION_TYPE <void (Job&)> mJob;

src/ripple_core/functional/JobQueue.h

@@ -4,94 +4,53 @@
 */
 //==============================================================================
 
-#ifndef RIPPLE_JOBQUEUE_H_INCLUDED
-#define RIPPLE_JOBQUEUE_H_INCLUDED
+#ifndef RIPPLE_CORE_JOBQUEUE_H_INCLUDED
+#define RIPPLE_CORE_JOBQUEUE_H_INCLUDED
 
-class JobQueue : private Workers::Callback
+class JobQueue : public Service
 {
+protected:
+    JobQueue (char const* name, Service& parent);
+
 public:
-    // Statistics on a particular JobType
-    struct Count
-    {
-        Count () noexcept;
-        explicit Count (JobType type) noexcept;
+    static JobQueue* New (Service& parent, Journal journal);
 
-        JobType type;    // The type of Job these counts reflect
-        int waiting;     // The number waiting
-        int running;     // How many are running
-        int deferred;    // Number of jobs we didn't signal due to limits
-    };
-
-    typedef std::map <JobType, Count> JobCounts;
-
-    //--------------------------------------------------------------------------
-
-    JobQueue ();
-
-    ~JobQueue ();
+    virtual ~JobQueue () { }
 
     // VFALCO TODO make convenience functions that allow the caller to not 
     //             have to call bind.
     //
-    void addJob (JobType type, const std::string& name, const FUNCTION_TYPE<void (Job&)>& job);
+    virtual void addJob (JobType type, const std::string& name, const FUNCTION_TYPE<void (Job&)>& job) = 0;
 
-    int getJobCount (JobType t);        // Jobs waiting at this priority
+    // Jobs waiting at this priority
+    virtual int getJobCount (JobType t) = 0;
 
-    int getJobCountTotal (JobType t);   // Jobs waiting plus running at this priority
+    // Jobs waiting plus running at this priority
+    virtual int getJobCountTotal (JobType t) = 0;
 
-    int getJobCountGE (JobType t);      // All waiting jobs at or greater than this priority
+    // All waiting jobs at or greater than this priority
+    virtual int getJobCountGE (JobType t) = 0;
 
-    std::vector< std::pair<JobType, std::pair<int, int> > > getJobCounts (); // jobs waiting, threads doing
+    // jobs waiting, threads doing
+    virtual std::vector< std::pair<JobType, std::pair<int, int> > > getJobCounts () = 0;
 
-    void shutdown ();
+    virtual void shutdown () = 0;
 
-    void setThreadCount (int c, bool const standaloneMode);
+    virtual void setThreadCount (int c, bool const standaloneMode) = 0;
 
     // VFALCO TODO Rename these to newLoadEventMeasurement or something similar
     //             since they create the object.
     //
-    LoadEvent::pointer getLoadEvent (JobType t, const std::string& name)
-    {
-        return boost::make_shared<LoadEvent> (boost::ref (mJobLoads[t]), name, true);
-    }
+    virtual LoadEvent::pointer getLoadEvent (JobType t, const std::string& name) = 0;
 
     // VFALCO TODO Why do we need two versions, one which returns a shared
     //             pointer and the other which returns an autoptr?
     //          
-    LoadEvent::autoptr getLoadEventAP (JobType t, const std::string& name)
-    {
-        return LoadEvent::autoptr (new LoadEvent (mJobLoads[t], name, true));
-    }
+    virtual LoadEvent::autoptr getLoadEventAP (JobType t, const std::string& name) = 0;
 
-    bool isOverloaded ();
+    virtual bool isOverloaded () = 0;
 
-    Json::Value getJson (int c = 0);
-
-private:
-    typedef std::set <Job> JobSet;
-
-    struct State
-    {
-        State ();
-
-        uint64 lastJob;
-        JobSet jobSet;
-        JobCounts jobCounts;
-    };
-
-    void queueJob (Job const& job, ScopedLock const& lock);
-    void getNextJob (Job& job, ScopedLock const& lock);
-    void finishJob (Job const& job);
-    void processTask ();
-
-    static int getJobLimit (JobType type);
-
-private:
-    typedef CriticalSection::ScopedLockType ScopedLock;
-    CriticalSection m_mutex;
-    State m_state;
-    Workers m_workers;
-    LoadMonitor mJobLoads [NUM_JOB_TYPES];
+    virtual Json::Value getJson (int c = 0) = 0;
 };
 
 #endif

src/ripple_core/node/HyperLevelDBBackendFactory.cpp

@@ -193,6 +193,11 @@ public:
         return m_batch.getWriteLoad ();
     }
 
+    void stopAsync ()
+    {
+        m_batch.stopAsync();
+    }
+
     //--------------------------------------------------------------------------
 
     void writeBatch (NodeStore::Batch const& batch)
@@ -200,6 +205,11 @@ public:
         storeBatch (batch);
     }
 
+    void writeStopped ()
+    {
+        m_scheduler.scheduledTasksStopped ();
+    }
+
 private:
     size_t const m_keyBytes;
     NodeStore::Scheduler& m_scheduler;

src/ripple_core/node/KeyvaDBBackendFactory.cpp

@@ -134,6 +134,11 @@ public:
         return 0;
     }
 
+    void stopAsync ()
+    {
+        m_scheduler.scheduledTasksStopped ();
+    }
+
     //--------------------------------------------------------------------------
 
 private:

src/ripple_core/node/LevelDBBackendFactory.cpp

@@ -193,6 +193,11 @@ public:
         return m_batch.getWriteLoad ();
     }
 
+    void stopAsync ()
+    {
+        m_batch.stopAsync ();
+    }
+
     //--------------------------------------------------------------------------
 
     void writeBatch (NodeStore::Batch const& batch)
@@ -200,6 +205,11 @@ public:
         storeBatch (batch);
     }
 
+    void writeStopped ()
+    {
+        m_scheduler.scheduledTasksStopped ();
+    }
+
 private:
     size_t const m_keyBytes;
     NodeStore::Scheduler& m_scheduler;

src/ripple_core/node/MdbBackendFactory.cpp

@@ -229,6 +229,11 @@ public:
         return m_batch.getWriteLoad ();
     }
 
+    void stopAsync ()
+    {
+        m_batch.stopAsync();
+    }
+
     //--------------------------------------------------------------------------
 
     void writeBatch (Batch const& batch)
@@ -236,6 +241,11 @@ public:
         storeBatch (batch);
     }
 
+    void writeStopped ()
+    {
+        m_scheduler.scheduledTasksStopped ();
+    }
+
 private:
     size_t const m_keyBytes;
     NodeStore::Scheduler& m_scheduler;

src/ripple_core/node/MemoryBackendFactory.cpp

@@ -10,8 +10,10 @@ private:
     typedef std::map <uint256 const, NodeObject::Ptr> Map;
 
 public:
-    Backend (size_t keyBytes, StringPairArray const& keyValues)
+    Backend (size_t keyBytes, StringPairArray const& keyValues,
+        NodeStore::Scheduler& scheduler)
         : m_keyBytes (keyBytes)
+        , m_scheduler (scheduler)
     {
     }
 
@@ -71,12 +73,18 @@ public:
         return 0;
     }
 
+    void stopAsync ()
+    {
+        m_scheduler.scheduledTasksStopped ();
+    }
+
     //--------------------------------------------------------------------------
 
 private:
     size_t const m_keyBytes;
 
     Map m_map;
+    NodeStore::Scheduler& m_scheduler;
 };
 
 //------------------------------------------------------------------------------
@@ -104,7 +112,7 @@ NodeStore::Backend* MemoryBackendFactory::createInstance (
     StringPairArray const& keyValues,
     NodeStore::Scheduler& scheduler)
 {
-    return new MemoryBackendFactory::Backend (keyBytes, keyValues);
+    return new MemoryBackendFactory::Backend (keyBytes, keyValues, scheduler);
 }
 
 //------------------------------------------------------------------------------

src/ripple_core/node/NodeStore.cpp

@@ -173,7 +173,7 @@ void NodeStore::BatchWriter::store (NodeObject::ref object)
     {
         mWritePending = true;
 
-        m_scheduler.scheduleTask (this);
+        m_scheduler.scheduleTask (*this);
     }
 }
 
@@ -184,6 +184,14 @@ int NodeStore::BatchWriter::getWriteLoad ()
     return std::max (mWriteLoad, static_cast<int> (mWriteSet.size ()));
 }
 
+void NodeStore::BatchWriter::stopAsync ()
+{
+    LockType::scoped_lock sl (mWriteMutex);
+
+    if (! mWritePending)
+        m_callback.writeStopped ();
+}
+
 void NodeStore::BatchWriter::performScheduledTask ()
 {
     writeBatch ();
@@ -212,6 +220,8 @@ void NodeStore::BatchWriter::writeBatch ()
                 mWritePending = false;
                 mWriteLoad = 0;
 
+                m_callback.writeStopped ();
+
                 // VFALCO NOTE Fix this function to not return from the middle
                 return;
             }
@@ -241,10 +251,13 @@ class NodeStoreImp
     , LeakChecked <NodeStoreImp>
 {
 public:
-    NodeStoreImp (Parameters const& backendParameters,
+    NodeStoreImp (char const* name,
+                  Service& parent,
+                  Parameters const& backendParameters,
                   Parameters const& fastBackendParameters,
                   Scheduler& scheduler)
-        : m_scheduler (scheduler)
+        : NodeStore (name, parent)
+        , m_scheduler (scheduler)
         , m_backend (createBackend (backendParameters, scheduler))
         , m_fastBackend ((fastBackendParameters.size () > 0)
             ? createBackend (fastBackendParameters, scheduler) : nullptr)
@@ -484,6 +497,19 @@ public:
         sourceDatabase.visitAll (callback);
     }
 
+    //------------------------------------------------------------------------------
+    //
+    // Service
+
+    void onServiceStop ()
+    {
+        // notify the Backend
+        m_backend->stopAsync();
+
+        if (m_fastBackend)
+            m_fastBackend->stopAsync();
+    }
+
     //------------------------------------------------------------------------------
 
     static void missing_backend ()
@@ -551,9 +577,13 @@ NodeStore::Scheduler& NodeStore::getSynchronousScheduler ()
     // Simple scheduler that performs the task immediately
     struct SynchronousScheduler : Scheduler
     {
-        void scheduleTask (Task* task)
+        void scheduleTask (Task& task)
+        {
+            task.performScheduledTask ();
+        }
+
+        void scheduledTasksStopped ()
         {
-            task->performScheduledTask ();
         }
     };
 
@@ -583,11 +613,24 @@ void NodeStore::addAvailableBackends ()
     NodeStore::addBackendFactory (KeyvaDBBackendFactory::getInstance ());
 }
 
-NodeStore* NodeStore::New (Parameters const& backendParameters,
+//------------------------------------------------------------------------------
+
+NodeStore::NodeStore (char const* name, Service& parent)
+    : Service (name, parent)
+{
+}
+
+//------------------------------------------------------------------------------
+
+NodeStore* NodeStore::New (char const* name,
+                           Service& parent,
+                           Parameters const& backendParameters,
                            Parameters fastBackendParameters,
                            Scheduler& scheduler)
 {
-    return new NodeStoreImp (backendParameters,
+    return new NodeStoreImp (name,
+                             parent,
+                             backendParameters,
                              fastBackendParameters,
                              scheduler);
 }
@@ -1032,10 +1075,17 @@ static NodeStoreTimingTests nodeStoreTimingTests;
 class NodeStoreTests : public NodeStoreUnitTest
 {
 public:
-    NodeStoreTests () : NodeStoreUnitTest ("NodeStore")
+    NodeStoreTests ()
+        : NodeStoreUnitTest ("NodeStore")
     {
     }
 
+    ~NodeStoreTests ()
+    {
+    }
+
+    //--------------------------------------------------------------------------
+
     void testImport (String destBackendType, String srcBackendType, int64 seedValue)
     {
         File const node_db (File::createTempFile ("node_db"));
@@ -1049,7 +1099,8 @@ public:
 
         // Write to source db
         {
-            ScopedPointer <NodeStore> src (NodeStore::New (srcParams));
+            ScopedService service ("test");
+            ScopedPointer <NodeStore> src (NodeStore::New ("test", service, srcParams));
 
             storeBatch (*src, batch);
         }
@@ -1057,8 +1108,10 @@ public:
         NodeStore::Batch copy;
 
         {
+            ScopedService service ("test");
+
             // Re-open the db
-            ScopedPointer <NodeStore> src (NodeStore::New (srcParams));
+            ScopedPointer <NodeStore> src (NodeStore::New ("test", service, srcParams));
 
             // Set up the destination database
             File const dest_db (File::createTempFile ("dest_db"));
@@ -1066,7 +1119,7 @@ public:
             destParams.set ("type", destBackendType);
             destParams.set ("path", dest_db.getFullPathName ());
 
-            ScopedPointer <NodeStore> dest (NodeStore::New (destParams));
+            ScopedPointer <NodeStore> dest (NodeStore::New ("test", service, destParams));
 
             beginTestCase (String ("import into '") + destBackendType + "' from '" + srcBackendType + "'");
 
@@ -1116,8 +1169,10 @@ public:
         createPredictableBatch (batch, 0, numObjectsToTest, seedValue);
 
         {
+            ScopedService service ("test");
+
             // Open the database
-            ScopedPointer <NodeStore> db (NodeStore::New (nodeParams, tempParams));
+            ScopedPointer <NodeStore> db (NodeStore::New ("test", service, nodeParams, tempParams));
 
             // Write the batch
             storeBatch (*db, batch);
@@ -1141,8 +1196,10 @@ public:
         if (testPersistence)
         {
             {
+                ScopedService service ("test");
+
                 // Re-open the database without the ephemeral DB
-                ScopedPointer <NodeStore> db (NodeStore::New (nodeParams));
+                ScopedPointer <NodeStore> db (NodeStore::New ("test", service, nodeParams));
 
                 // Read it back in
                 NodeStore::Batch copy;
@@ -1156,8 +1213,11 @@ public:
 
             if (useEphemeralDatabase)
             {
+                ScopedService service ("test");
+
                 // Verify the ephemeral db
-                ScopedPointer <NodeStore> db (NodeStore::New (tempParams, StringPairArray ()));
+                ScopedPointer <NodeStore> db (NodeStore::New ("test",
+                    service, tempParams, StringPairArray ()));
 
                 // Read it back in
                 NodeStore::Batch copy;

src/ripple_core/node/NodeStore.h

@@ -20,8 +20,11 @@
 
     @see NodeObject
 */
-class NodeStore
+class NodeStore : public Service
 {
+protected:
+    NodeStore (char const* name, Service& parent);
+
 public:
     enum
     {
@@ -114,7 +117,6 @@ public:
             virtual ~Task () { }
 
             /** Performs the task.
-
                 The call may take place on a foreign thread.
             */
             virtual void performScheduledTask () = 0;
@@ -125,7 +127,10 @@ public:
             Depending on the implementation, this could happen
             immediately or get deferred.
         */
-        virtual void scheduleTask (Task* task) = 0;
+        virtual void scheduleTask (Task& task) = 0;
+
+        /** Notifies the scheduler that all tasks are complete. */
+        virtual void scheduledTasksStopped () = 0;
     };
 
     //--------------------------------------------------------------------------
@@ -149,6 +154,9 @@ public:
         struct Callback
         {
             virtual void writeBatch (Batch const& batch) = 0;
+
+            // Called after stopAsync when there is no more pending write
+            virtual void writeStopped () = 0;
         };
 
         /** Create a batch writer. */
@@ -170,6 +178,9 @@ public:
         /** Get an estimate of the amount of writing I/O pending. */
         int getWriteLoad ();
 
+        /** Called to notify that the NodeStore wants to stop. */
+        void stopAsync ();
+
     private:
         void performScheduledTask ();
         void writeBatch ();
@@ -280,6 +291,9 @@ public:
 
         /** Estimate the number of write operations pending. */
         virtual int getWriteLoad () = 0;
+
+        /** Called to notify the Backend that the NodeStore wants to stop. */
+        virtual void stopAsync () = 0;
     };
 
     //--------------------------------------------------------------------------
@@ -331,7 +345,9 @@ public:
 
         @return The opened database.
     */
-    static NodeStore* New (Parameters const& backendParameters,
+    static NodeStore* New (char const* name,
+                           Service& parent,
+                           Parameters const& backendParameters,
                            Parameters fastBackendParameters = Parameters (),
                            Scheduler& scheduler = getSynchronousScheduler ());
 

src/ripple_core/node/NullBackendFactory.cpp

@@ -7,7 +7,8 @@
 class NullBackendFactory::Backend : public NodeStore::Backend
 {
 public:
-    Backend ()
+    explicit Backend (NodeStore::Scheduler& scheduler)
+        : m_scheduler (scheduler)
     {
     }
 
@@ -41,6 +42,14 @@ public:
     {
         return 0;
     }
+
+    void stopAsync ()
+    {
+        m_scheduler.scheduledTasksStopped ();
+    }
+
+private:
+    NodeStore::Scheduler& m_scheduler;
 };
 
 //------------------------------------------------------------------------------
@@ -66,9 +75,9 @@ String NullBackendFactory::getName () const
 NodeStore::Backend* NullBackendFactory::createInstance (
     size_t,
     StringPairArray const&,
-    NodeStore::Scheduler&)
+    NodeStore::Scheduler& scheduler)
 {
-    return new NullBackendFactory::Backend;
+    return new NullBackendFactory::Backend (scheduler);
 }
 
 //------------------------------------------------------------------------------

src/ripple_core/ripple_core.h

@@ -4,17 +4,10 @@
 */
 //==============================================================================
 
-#ifndef RIPPLE_CORE_RIPPLEHEADER
-#define RIPPLE_CORE_RIPPLEHEADER
+#ifndef RIPPLE_CORE_H_INCLUDED
+#define RIPPLE_CORE_H_INCLUDED
 
-// For Validators
-//
-// VFALCO NOTE It is unfortunate that we are exposing boost/asio.hpp
-//             needlessly. Its only required because of the buffers types.
-//             The HTTPClient interface doesn't need asio (although the
-//             implementation does. This is also reuqired for
-//             UniformResourceLocator.
-//
+// VFALCO TODO For UniformResourceLocator, remove asap
 #include "beast/modules/beast_asio/beast_asio.h"
 
 #include "../ripple_basics/ripple_basics.h"