Merge pull request #1367 from joto/flat-node-file-option

Flat node file option

Author: lonvia

src/db-check.cpp

@@ -42,7 +42,7 @@ void check_db(options_t const &options)
         // it probably means we used a flat node store when we created this
         // database. Check for that and stop if it looks like we are missing
         // the node location store option.
-        if (options.append && !options.flat_node_cache_enabled) {
+        if (options.append && options.flat_node_file.empty()) {
             if (!has_table(db_connection, options.middle_dbschema,
                            options.prefix + "_nodes")) {
                 throw std::runtime_error{

src/middle-pgsql.cpp

@@ -318,7 +318,7 @@ void middle_pgsql_t::node_set(osmium::Node const &node)
 {
     m_cache->set(node.id(), node.location());
 
-    if (m_options->flat_node_cache_enabled) {
+    if (!m_options->flat_node_file.empty()) {
         m_persistent_cache->set(node.id(), node.location());
     } else {
         m_db_copy.new_line(m_tables[NODE_TABLE].copy_target());
@@ -359,7 +359,7 @@ void middle_pgsql_t::node_delete(osmid_t osm_id)
 {
     assert(m_options->append);
 
-    if (m_options->flat_node_cache_enabled) {
+    if (!m_options->flat_node_file.empty()) {
         m_persistent_cache->set(osm_id, osmium::Location{});
     } else {
         m_db_copy.new_line(m_tables[NODE_TABLE].copy_target());
@@ -623,7 +623,7 @@ void middle_pgsql_t::flush() { m_db_copy.sync(); }
 void middle_pgsql_t::stop(thread_pool_t &pool)
 {
     m_cache.reset();
-    if (m_options->flat_node_cache_enabled) {
+    if (!m_options->flat_node_file.empty()) {
         m_persistent_cache.reset();
     }
 
@@ -774,8 +774,9 @@ middle_pgsql_t::middle_pgsql_t(options_t const *options)
       std::make_shared<db_copy_thread_t>(options->database_options.conninfo())),
   m_db_copy(m_copy_thread)
 {
-    if (options->flat_node_cache_enabled) {
-        m_persistent_cache.reset(new node_persistent_cache{options});
+    if (!options->flat_node_file.empty()) {
+        m_persistent_cache.reset(new node_persistent_cache{
+            options->flat_node_file, options->droptemp});
     }
 
     log_debug("Mid: pgsql, cache={}", options->cache);
@@ -789,7 +790,7 @@ middle_pgsql_t::middle_pgsql_t(options_t const *options)
     }
 
     m_tables[NODE_TABLE] =
-        table_desc{*options, sql_for_nodes(!options->flat_node_cache_enabled)};
+        table_desc{*options, sql_for_nodes(options->flat_node_file.empty())};
     m_tables[WAY_TABLE] =
         table_desc{*options, sql_for_ways(has_bucket_index,
                                           options->way_node_index_id_shift)};

src/middle-pgsql.hpp

@@ -15,6 +15,7 @@
 #include "middle.hpp"
 #include "node-persistent-cache.hpp"
 #include "node-ram-cache.hpp"
+#include "options.hpp"
 #include "pgsql.hpp"
 
 class middle_query_pgsql_t : public middle_query_t

src/node-persistent-cache.cpp

@@ -2,7 +2,8 @@
 
 #include "logging.hpp"
 #include "node-persistent-cache.hpp"
-#include "options.hpp"
+
+#include <cassert>
 
 void node_persistent_cache::set(osmid_t id, osmium::Location location)
 {
@@ -15,15 +16,12 @@ osmium::Location node_persistent_cache::get(osmid_t id) const noexcept
         static_cast<osmium::unsigned_object_id_type>(id));
 }
 
-node_persistent_cache::node_persistent_cache(const options_t *options)
+node_persistent_cache::node_persistent_cache(std::string const &file_name, bool remove_file)
 {
-    if (!options->flat_node_file) {
-        throw std::runtime_error{"Unable to set up persistent cache: the name "
-                                 "of the flat node file was not set."};
-    }
+    assert(!file_name.empty());
 
-    m_fname = options->flat_node_file->c_str();
-    m_remove_file = options->droptemp;
+    m_fname = file_name.c_str();
+    m_remove_file = remove_file;
     log_debug("Mid: loading persistent node cache from {}", m_fname);
 
     // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-signed-bitwise)

src/node-persistent-cache.hpp

@@ -8,12 +8,10 @@
 
 #include "osmtypes.hpp"
 
-class options_t;
-
 class node_persistent_cache
 {
 public:
-    node_persistent_cache(options_t const *options);
+    node_persistent_cache(std::string const &file_name, bool remove_file);
     ~node_persistent_cache() noexcept;
 
     void set(osmid_t id, osmium::Location location);

src/options.cpp

@@ -544,7 +544,6 @@ options_t::options_t(int argc, char *argv[]) : options_t()
             droptemp = true;
             break;
         case 'F':
-            flat_node_cache_enabled = true;
             flat_node_file = optarg;
             break;
         case 211:
@@ -702,7 +701,7 @@ void options_t::check_options()
             throw std::runtime_error{
                 "RAM node cache can only be disable in slim mode."};
         }
-        if (!flat_node_cache_enabled) {
+        if (flat_node_file.empty()) {
             log_warn("RAM cache is disabled. This will likely slow down "
                      "processing a lot.");
         }

src/options.hpp

@@ -114,9 +114,10 @@ class options_t
     /// only copy rows that match an explicitly listed key
     bool hstore_match_only = false;
 
-    bool flat_node_cache_enabled = false;
     bool reproject_area = false;
-    boost::optional<std::string> flat_node_file{boost::none};
+
+    /// Name of the flat node file used. Empty if flat node file is not enabled.
+    std::string flat_node_file{};
 
     /**
      * these options allow you to control the name of the

tests/common-options.hpp

@@ -66,9 +66,7 @@ class opt_t
 
     opt_t &flatnodes()
     {
-        m_opt.flat_node_file =
-            boost::optional<std::string>("test_middle_flat.flat.nodes.bin");
-        m_opt.flat_node_cache_enabled = true;
+        m_opt.flat_node_file = "test_middle_flat.flat.nodes.bin";
         return *this;
     }
 

tests/test-middle.cpp

@@ -184,8 +184,7 @@ TEMPLATE_TEST_CASE("middle import", "", options_slim_default,
                    options_ram_optimized, options_ram_flatnode)
 {
     options_t const options = TestType::options(db);
-    testing::cleanup::file_t flatnode_cleaner{
-        options.flat_node_file.get_value_or("")};
+    testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
 
     auto conn = db.connect();
     auto const num_tables =
@@ -373,8 +372,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update node", "",
 {
     options_t options = TestType::options(db);
 
-    testing::cleanup::file_t flatnode_cleaner{
-        options.flat_node_file.get_value_or("")};
+    testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
 
     // Prepare a buffer with some nodes which we will add and change.
     test_buffer_t buffer;
@@ -573,8 +571,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update way", "",
 {
     options_t options = TestType::options(db);
 
-    testing::cleanup::file_t flatnode_cleaner{
-        options.flat_node_file.get_value_or("")};
+    testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
 
     // Create some ways we'll use for the tests.
     test_buffer_t buffer;
@@ -730,8 +727,7 @@ TEMPLATE_TEST_CASE("middle: add way with attributes", "", options_slim_default,
     SECTION("With attributes") { options.extra_attributes = true; }
     SECTION("No attributes") { options.extra_attributes = false; }
 
-    testing::cleanup::file_t flatnode_cleaner{
-        options.flat_node_file.get_value_or("")};
+    testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
 
     // Create some ways we'll use for the tests.
     test_buffer_t buffer;
@@ -821,8 +817,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update relation", "",
 {
     options_t options = TestType::options(db);
 
-    testing::cleanup::file_t flatnode_cleaner{
-        options.flat_node_file.get_value_or("")};
+    testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
 
     // Create some relations we'll use for the tests.
     test_buffer_t buffer;
@@ -983,8 +978,7 @@ TEMPLATE_TEST_CASE("middle: add relation with attributes", "",
     SECTION("With attributes") { options.extra_attributes = true; }
     SECTION("No attributes") { options.extra_attributes = false; }
 
-    testing::cleanup::file_t flatnode_cleaner{
-        options.flat_node_file.get_value_or("")};
+    testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
 
     // Create some relations we'll use for the tests.
     test_buffer_t buffer;
@@ -1046,8 +1040,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
 {
     options_t options = TestType::options(db);
 
-    testing::cleanup::file_t flatnode_cleaner{
-        options.flat_node_file.get_value_or("")};
+    testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
 
     // create some nodes and ways we'll use for the tests
     test_buffer_t buffer;
@@ -1187,8 +1180,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in relation", "", options_slim_default,
 {
     options_t options = TestType::options(db);
 
-    testing::cleanup::file_t flatnode_cleaner{
-        options.flat_node_file.get_value_or("")};
+    testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
 
     // create some nodes, ways, and relations we'll use for the tests
     test_buffer_t buffer;

tests/test-persistent-cache.cpp

@@ -1,10 +1,8 @@
 #include <catch.hpp>
 
 #include "node-persistent-cache.hpp"
-#include "options.hpp"
 
 #include "common-cleanup.hpp"
-#include "common-options.hpp"
 
 static void write_and_read_location(node_persistent_cache &cache, osmid_t id,
                                     double x, double y)
@@ -27,12 +25,12 @@ static void delete_location(node_persistent_cache &cache, osmid_t id)
 
 TEST_CASE("Persistent cache", "[NoDB]")
 {
-    options_t const options = testing::opt_t().flatnodes();
-    testing::cleanup::file_t flatnode_cleaner{options.flat_node_file.get()};
+    std::string const flat_node_file = "test_middle_flat.flat.nodes.bin";
+    testing::cleanup::file_t flatnode_cleaner{flat_node_file};
 
     // create a new cache
     {
-        node_persistent_cache cache{&options};
+        node_persistent_cache cache{flat_node_file, false};
 
         // write in order
         write_and_read_location(cache, 10, 10.01, -45.3);
@@ -55,7 +53,7 @@ TEST_CASE("Persistent cache", "[NoDB]")
 
     // reopen the cache
     {
-        node_persistent_cache cache{&options};
+        node_persistent_cache cache{flat_node_file, false};
 
         // read all previously written locations
         read_location(cache, 10, 10.01, -45.3);