Remove all temporary files that are created by the IndexBuilder

src/index/Index.cpp

@@ -48,36 +48,14 @@ VocabularyData Index::createIdTriplesAndVocab(const string& ntFile) {
         _onDiskBase + EXTERNAL_LITS_TEXT_FILE_NAME,
         _onDiskBase + ".literals-index");
   }
+  deleteTemporaryFile(_onDiskBase + EXTERNAL_LITS_TEXT_FILE_NAME);
   // clear vocabulary to save ram (only information from partial binary files
   // used from now on). This will preserve information about externalized
   // Prefixes etc.
   _vocab.clear();
   convertPartialToGlobalIds(*vocabData.idTriples, vocabData.actualPartialSizes,
                             NUM_TRIPLES_PER_PARTIAL_VOCAB);
 
-  if (!_keepTempFiles) {
-    // remove temporary files only used during index creation
-    LOG(INFO) << "Removing temporary files (partial vocabulary and external "
-                 "text file...\n";
-
-    // TODO(all): using system and rm is not really elegant nor portable.
-    // use std::filesystem as soon as QLever is ported to C++17
-    string removeCommand1 =
-        "rm -- " + _onDiskBase + EXTERNAL_LITS_TEXT_FILE_NAME;
-    bool w1 = system(removeCommand1.c_str());
-    string removeCommand2 =
-        "rm -- " + _onDiskBase + PARTIAL_VOCAB_FILE_NAME + "*";
-    bool w2 = system(removeCommand2.c_str());
-    if (w1 || w2) {
-      LOG(INFO)
-          << "Warning. Deleting of temporary files probably not successful\n";
-    } else {
-      LOG(INFO) << "Done.\n";
-    }
-  } else {
-    LOG(INFO) << "Keeping temporary files (partial vocabulary and external "
-                 "text file...\n";
-  }
   return vocabData;
 }
 
@@ -232,6 +210,11 @@ VocabularyData Index::passFileForVocabulary(const string& filename,
   VocabularyData res;
   res.nofWords = mergeVocabulary(_onDiskBase, numFiles, &res.langPredLowerBound,
                                  &res.langPredUpperBound);
+  for (size_t i = 0; i < numFiles; ++i) {
+    string partialFilename =
+        _onDiskBase + PARTIAL_VOCAB_FILE_NAME + std::to_string(i);
+    deleteTemporaryFile(partialFilename);
+  }
   res.idTriples = std::move(idTriples);
   res.actualPartialSizes = std::move(actualPartialSizes);
   LOG(INFO) << "Pass done.\n";
@@ -257,6 +240,9 @@ void Index::convertPartialToGlobalIds(
     LOG(INFO) << "Reading IdMap from " << mmapFilename << " ...\n";
     ad_utility::HashMap<Id, Id> idMap = IdMapFromPartialIdMapFile(mmapFilename);
     LOG(INFO) << "Done reading idMap\n";
+    // Delete the temporary file in which we stored this map
+    deleteTemporaryFile(mmapFilename);
+
     // update the triples for which this partial vocabulary was responsible
     for (size_t tmpNum = 0; tmpNum < actualLinesPerPartial[partialNum];
          ++tmpNum) {
@@ -586,9 +572,9 @@ void Index::createPatternsImpl(const string& fileName,
   LOG(DEBUG) << "Pattern set size: " << patternSet.size() << std::endl;
 
   // Associate entities with patterns if possible, store has-relation otherwise
-  ad_utility::MmapVector<std::array<Id, 2>> entityHasPattern(
+  ad_utility::MmapVectorTmp<std::array<Id, 2>> entityHasPattern(
       0, fileName + ".mmap.entityHasPattern.tmp");
-  ad_utility::MmapVector<std::array<Id, 2>> entityHasPredicate(
+  ad_utility::MmapVectorTmp<std::array<Id, 2>> entityHasPredicate(
       0, fileName + ".mmap.entityHasPredicate.tmp");
 
   size_t numEntitiesWithPatterns = 0;

src/index/Index.h

@@ -713,4 +713,14 @@ class Index {
     }
     LOG(INFO) << "Done" << std::endl;
   }
+
+  /**
+   * Delete a temporary file unless the _keepTempFiles flag is set
+   * @param path
+   */
+  void deleteTemporaryFile(const string& path) {
+    if (!_keepTempFiles) {
+      ad_utility::deleteFile(path);
+    }
+  }
 };

src/index/VocabularyGenerator.cpp

@@ -41,7 +41,7 @@ class QueueCompare {
 // ___________________________________________________________________
 size_t mergeVocabulary(const std::string& basename, size_t numFiles,
                        Id* langPredLowerBound, Id* langPredUpperBound) {
-  std::vector<std::fstream> infiles;
+  std::vector<std::ifstream> infiles;
 
   // we will store pairs of <partialId, globalId>
   std::vector<IdPairMMapVec> idVecs;
@@ -56,8 +56,8 @@ size_t mergeVocabulary(const std::string& basename, size_t numFiles,
 
   // open and prepare all infiles and mmap output vectors
   for (size_t i = 0; i < numFiles; i++) {
-    infiles.emplace_back(basename + PARTIAL_VOCAB_FILE_NAME + std::to_string(i),
-                         std::ios_base::in | std::ios_base::out);
+    infiles.emplace_back(basename + PARTIAL_VOCAB_FILE_NAME +
+                         std::to_string(i));
     idVecs.emplace_back(0, basename + PARTIAL_MMAP_IDS + std::to_string(i));
     AD_CHECK(infiles.back().is_open());
 

src/util/BufferedVector.h

@@ -14,6 +14,7 @@ namespace ad_utility {
 // is big The switching threshold can be set when creating the vector Currently
 // only push_back and clear are supported (+ all kinds of access to existing
 // elements) Can be trivially extended to complete the interface
+// The backup file is temporary and will be deleted in the destructor
 template <class T>
 class BufferedVector {
  public:
@@ -109,6 +110,6 @@ class BufferedVector {
 
   // the two possible data storages
   std::vector<T> _vec;
-  ad_utility::MmapVector<T> _extVec;
+  ad_utility::MmapVectorTmp<T> _extVec;
 };
 }  // namespace ad_utility

src/util/File.h

@@ -15,6 +15,7 @@
 #include <vector>
 
 #include "./Exception.h"
+#include "Log.h"
 
 using std::cerr;
 using std::cout;
@@ -244,4 +245,19 @@ class File {
     return (stat(path.c_str(), &buffer) == 0);
   }
 };
+
+/**
+ * @brief Delete the file at a given path
+ * Currently uses the linux rm command until we support std::filesystem
+ * @param path
+ */
+inline void deleteFile(const string& path) {
+  // TODO<all>: As soon as we have GCC 8, we can use std::filesystem
+  string command = "rm -- " + path;
+  if (system(command.c_str())) {
+    LOG(WARN) << "Deletion of file " << path << " was probably not successful"
+              << std::endl;
+  }
+}
+
 }  // namespace ad_utility

src/util/MmapVector.h

@@ -10,6 +10,7 @@
 #include <fstream>
 #include <sstream>
 #include "../util/Exception.h"
+#include "File.h"
 
 using std::string;
 
@@ -332,5 +333,25 @@ class MmapVectorView : private MmapVector<T> {
   // _____________________________________________________________
   std::string getFilename() const { return this->_filename; }
 };
+
+// MmapVector that deletes the underlying file on destruction.
+// This is the only difference to the ordinary MmapVector(which is persistent)
+template <class T>
+class MmapVectorTmp : public MmapVector<T> {
+ public:
+  using MmapVector<T>::MmapVector;
+
+  // If we still own a file, delete it after cleaning up
+  // everything else
+  ~MmapVectorTmp() {
+    // if the filename is not empty, we still own a file
+    std::string oldFilename = this->_filename;
+    this->close();
+    if (!oldFilename.empty()) {
+      ad_utility::deleteFile(oldFilename);
+    }
+  }
+};
+
 }  // namespace ad_utility
 #include "./MmapVectorImpl.h"