[FOLD] Code review changes; SAH singleton refactor pending

src/ripple/app/main/Application.cpp

@@ -355,6 +355,7 @@ class ApplicationImp
     std::unique_ptr <LedgerMaster> m_ledgerMaster;
     std::unique_ptr <InboundLedgers> m_inboundLedgers;
     std::unique_ptr <InboundTransactions> m_inboundTransactions;
+    std::unique_ptr <RPC::ShardArchiveHandler> m_shardArchiveHandler;
     TaggedCache <uint256, AcceptedLedger> m_acceptedLedgerCache;
     std::unique_ptr <NetworkOPs> m_networkOPs;
     std::unique_ptr <Cluster> cluster_;
@@ -683,6 +684,11 @@ class ApplicationImp
         return *m_inboundTransactions;
     }
 
+    RPC::ShardArchiveHandler* shardArchiveHandler () override
+    {
+        return m_shardArchiveHandler.get();
+    }
+
     TaggedCache <uint256, AcceptedLedger>& getAcceptedLedgerCache () override
     {
         return m_acceptedLedgerCache;
@@ -1628,13 +1634,7 @@ bool ApplicationImp::setup()
                     *this,
                     *m_jobQueue);
 
-                if (!handler)
-                {
-                    JLOG(m_journal.fatal())
-                        << "Failed to create ShardArchiveHandler.";
-
-                    return false;
-                }
+                assert(handler);
 
                 if (!handler->initFromDB())
                 {

src/ripple/app/main/Application.h

@@ -37,6 +37,7 @@ namespace unl { class Manager; }
 namespace Resource { class Manager; }
 namespace NodeStore { class Database; class DatabaseShard; }
 namespace perf { class PerfLog; }
+namespace RPC { class ShardArchiveHandler; }
 
 // VFALCO TODO Fix forward declares required for header dependency loops
 class AmendmentTable;
@@ -147,6 +148,7 @@ class Application : public beast::PropertyStream::Source
     virtual NodeStore::DatabaseShard*   getShardStore() = 0;
     virtual InboundLedgers&             getInboundLedgers () = 0;
     virtual InboundTransactions&        getInboundTransactions () = 0;
+    virtual RPC::ShardArchiveHandler*   shardArchiveHandler () = 0;
 
     virtual
     TaggedCache <uint256, AcceptedLedger>&

src/ripple/net/DatabaseBody.h

@@ -64,6 +64,7 @@ class DatabaseBody::value_type
     std::condition_variable c_;
     uint64_t handler_count_ = 0;
     uint64_t part_ = 0;
+    bool closing_ = false;
 
 public:
 
@@ -77,7 +78,7 @@ class DatabaseBody::value_type
     bool
     is_open() const
     {
-        return bool {conn_ };
+        return bool{conn_};
     }
 
     /// Returns the size of the file if open
@@ -100,8 +101,8 @@ class DatabaseBody::value_type
         @param ec Set to the error, if any occurred
     */
     void
-    open(boost::filesystem::path path, Config const& config,
-        boost::asio::io_service& io_service, boost::system::error_code & ec);
+    open(boost::filesystem::path path, Config const & config,
+        boost::asio::io_service & io_service, boost::system::error_code & ec);
 };
 
 /** Algorithm for storing buffers when parsing.
@@ -111,10 +112,10 @@ class DatabaseBody::value_type
 */
 class DatabaseBody::reader
 {
-    value_type& body_;  // The body we are writing to
+    value_type & body_;  // The body we are writing to
 
-    static const uint32_t FLUSH_SIZE   = 50000000;
-    static const uint8_t  MAX_HANDLERS = 3;
+    static const uint32_t FLUSH_SIZE = 50000000;
+    static const uint8_t MAX_HANDLERS = 3;
     static const uint16_t MAX_ROW_SIZE_PAD = 500;
 
 public:
@@ -127,7 +128,7 @@ class DatabaseBody::reader
     //
     template<bool isRequest, class Fields>
     explicit
-    reader(boost::beast::http::header<isRequest, Fields>&h, value_type& b);
+    reader(boost::beast::http::header<isRequest, Fields> & h, value_type & b);
 
     // Initializer
     //
@@ -138,15 +139,16 @@ class DatabaseBody::reader
     // optionally use for optimization.
     //
     void
-    init(boost::optional<std::uint64_t> const&, boost::system::error_code& ec);
+    init(boost::optional<std::uint64_t> const &,
+        boost::system::error_code & ec);
 
     // This function is called one or more times to store
     // buffer sequences corresponding to the incoming body.
     //
     template<class ConstBufferSequence>
     std::size_t
-    put(ConstBufferSequence const& buffers,
-        boost::system::error_code& ec);
+    put(ConstBufferSequence const & buffers,
+        boost::system::error_code & ec);
 
     void
     do_put(std::string data);
@@ -159,7 +161,7 @@ class DatabaseBody::reader
     // would terminate the program.
     //
     void
-    finish(boost::system::error_code& ec);
+    finish(boost::system::error_code & ec);
 };
 
 } // ripple

src/ripple/net/DatabaseDownloader.h

@@ -31,9 +31,9 @@ class DatabaseDownloader : public SSLHTTPDownloader
 public:
 
     DatabaseDownloader(
-        boost::asio::io_service& io_service,
+        boost::asio::io_service & io_service,
         beast::Journal j,
-        Config const& config);
+        Config const & config);
 
 private:
 
@@ -45,15 +45,15 @@ class DatabaseDownloader : public SSLHTTPDownloader
         boost::system::error_code & ec) override;
 
     bool
-    checkPath(boost::filesystem::path const& dstPath) override;
+    checkPath(boost::filesystem::path const & dstPath) override;
 
     void
     closeBody(std::shared_ptr<parser> p) override;
 
     uint64_t
     size(std::shared_ptr<parser> p) override;
 
-    Config const& config_;
+    Config const & config_;
     boost::asio::io_service & io_service_;
 };
 

src/ripple/net/README.md → src/ripple/net/ShardDownloader.md

@@ -1,19 +1,21 @@
-# Shard Downloader Improvements
+# Shard Downloader Process
 
 ## Overview
 
-This document describes the changes made to the `SSLHTTPDownloader`, a class that performs the task of downloading shards from remote web servers via
+This document describes mechanics of the `SSLHTTPDownloader`, a class that performs the task of downloading shards from remote web servers via
 SSL HTTP. The downloader utilizes a strand (`boost::asio::io_service::strand`) to ensure that downloads are never executed concurrently. Hence, if a download is in progress when another download is initiated, the second download will be queued and invoked only when the first download is completed.
 
 ## New Features
 
+The downloader has been recently (March 2020) been modified to provide some key features:
+
 - The ability to stop downloads during a graceful shutdown.
 - The ability to resume partial downloads after a crash or shutdown.
 - <span style="color:gray">*(Deferred) The ability to download from multiple servers to a single file.*</span>
 
 ## Classes
 
-Implementing the shard downloader improvements required making changes to the following classes:
+Much of the shard downloading process concerns the following classes:
 
 - `SSLHTTPDownloader`
 
@@ -40,18 +42,17 @@ Implementing the shard downloader improvements required making changes to the fo
    start();
    ```
 
-   When a client submits a `download_shard` command via the RPC interface, each of the requested files is registered with the handler via the `add` method. After all the files have been registered, the handler's `start` method is invoked, which in turn creates an instance of the `SSLHTTPDownloader` and begins the first download. When the download is completed, the downloader invokes the handler's `complete` method, which will initiate the download of the next file, or simply return if there are no more downloads to process. When `complete` is invoked with no remaining files to be downloaded, the handler and downloader are longer destroyed automatically, but persist for the duration of the application.
+   When a client submits a `download_shard` command via the RPC interface, each of the requested files is registered with the handler via the `add` method. After all the files have been registered, the handler's `start` method is invoked, which in turn creates an instance of the `SSLHTTPDownloader` and begins the first download. When the download is completed, the downloader invokes the handler's `complete` method, which will initiate the download of the next file, or simply return if there are no more downloads to process. When `complete` is invoked with no remaining files to be downloaded, the handler and downloader  are not destroyed automatically, but persist for the duration of the application.
 
-Additionally, we create a new class to provide customized parsing for downloaded files:
+Additionally, we leverage a novel class to provide customized parsing for downloaded files:
 
 - `DatabaseBody`
 
    This class will define a custom message body type, allowing an `http::response_parser` to write to a SQLite database rather than to a flat file. This class is discussed in further detail in the Recovery section.
 
 ## Execution Concept
 
-This section describes in greater detail how the new features will be
-implemented in C++ using the `boost::asio` framework.
+This section describes in greater detail how the key features of the downloader are implemented in C++ using the `boost::asio` framework.
 
 ##### Member Variables:
 
@@ -62,7 +63,7 @@ will be used in the following code examples.
 using boost::asio::ssl::stream;
 using boost::asio::ip::tcp::socket;
 
-stream<socket>>         stream_;
+stream<socket>          stream_;
 std::condition_variable c_;
 std::atomic<bool>       isStopped_;
 ```
@@ -152,9 +153,9 @@ Although `SSLHTTPDownloader` is a generic class that could be used to download a
 
 | Index | URL                                 |
 |:-----:|:-----------------------------------:|
-| 1     | ht<span>tps://example.com/1.tar.lz4 |
-| 2     | ht<span>tps://example.com/2.tar.lz4 |
-| 5     | ht<span>tps://example.com/5.tar.lz4 |
+| 1     | ht<span />tps://example.com/1.tar.lz4 |
+| 2     | ht<span />tps://example.com/2.tar.lz4 |
+| 5     | ht<span />tps://example.com/5.tar.lz4 |
 
 ##### SSLHTTPDownloader
 
@@ -219,7 +220,7 @@ else
 
 ##### DatabaseBody
 
-Currently, the `SSLHTTPDownloader` leverages a `http::response_parser` instantiated with a `http::file_body`. The `file_body` class declares a nested type, `reader`, which does the task of writing HTTP message payloads (constituting a requested file) to the filesystem. In order for the `http::response_parser` to interface with the database, we implement a custom body type that declares a nested `reader` type which has been outfitted to persist octects received from the remote host to a local SQLite database. The code snippet below illustrates the customization points available to user-defined body types:
+Previously, the `SSLHTTPDownloader` leveraged an `http::response_parser` instantiated with an `http::file_body`. The `file_body` class declares a nested type, `reader`, which does the task of writing HTTP message payloads (constituting a requested file) to the filesystem. In order for the `http::response_parser` to interface with the database, we implement a custom body type that declares a nested `reader` type which has been outfitted to persist octects received from the remote host to a local SQLite database. The code snippet below illustrates the customization points available to user-defined body types:
 
 ```C++
 /// Defines a Body type