<p><b>@lonvia</b> requested changes on this pull request.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/osm2pgsql/pull/612#discussion_r161081385">node-persistent-cache.cpp</a>:</p>
<pre style='color:#555'>> +    fprintf(stderr, "Mid: removing persistent node cache at %s\n", fname);
+
+    // if we don't close the file, *NIX systems keep the inode and its blocks
+    // around
+    if (m_fd >= 0) {
+        close(m_fd);
+        m_fd = -1;
+    }
+
+    // but as the file is mmap'ed, we also have to release the shared_ptr
+    // which deletes the index_t, which unmmaps
+    m_index.reset();
+
+    if (remove_file) {
+        unlink(fname);
+    }
</pre>
<p>You are duplicating destructor code here. If you just move the three last lines to the end of the destructor, you can get rid of this entire function.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/osm2pgsql/pull/612#discussion_r161081909">node-persistent-cache.hpp</a>:</p>
<pre style='color:#555'>> @@ -32,6 +33,8 @@ class node_persistent_cache
     std::shared_ptr<node_ram_cache> m_ram_cache;
     int m_fd;
     std::unique_ptr<index_t> m_index;
+    bool remove_file;
+    const char *fname;
</pre>
<p>Coding style: follow the example and prefix members with m_.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/osm2pgsql/pull/612#discussion_r161083464">middle-pgsql.cpp</a>:</p>
<pre style='color:#555'>> @@ -1072,7 +1072,10 @@ void middle_pgsql_t::pgsql_stop_one(table_desc *table)
 void middle_pgsql_t::stop(void)
 {
     cache.reset();
-    if (out_options->flat_node_cache_enabled) persistent_cache.reset();
+    if (out_options->flat_node_cache_enabled) {
+        persistent_cache->clean_up();
+        persistent_cache.reset();
+    }
</pre>
<p>I still think that the solution should be generalized, so the benefit is there even without --drop. To achieve that the stop() function needs to be split at this point, the upper part executed before the indexing and the lower part needs to remain where the stop() call was before this change.</p>
<p>And if we do that, then the future vector below can even be merged with the one with the indexing functions in osmdata.cpp below, meaning that we get a much better parallelization.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/osm2pgsql/pull/612#pullrequestreview-88313231">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AD2-7r8zMDVdIFruIah91OxMRfakXNypks5tJoD7gaJpZM4JRzeA">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/AD2-7kQvEoFaKkh8aoIaeQaKbZgB7OnLks5tJoD7gaJpZM4JRzeA.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
  <link itemprop="url" href="https://github.com/openstreetmap/osm2pgsql/pull/612#pullrequestreview-88313231"></link>
  <meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/openstreetmap/osm2pgsql","title":"openstreetmap/osm2pgsql","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/openstreetmap/osm2pgsql"}},"updates":{"snippets":[{"icon":"PERSON","message":"@lonvia requested changes on #612"}],"action":{"name":"View Pull Request","url":"https://github.com/openstreetmap/osm2pgsql/pull/612#pullrequestreview-88313231"}}}</script>