From fe99b67f1464a6e246ca5add5da2b5c86c1efd6f Mon Sep 17 00:00:00 2001
From: Steven Murray <steven.murray@cern.ch>
Date: Tue, 7 Jun 2016 17:41:00 +0200
Subject: [PATCH] InMemoryCatalogue now uses a table to generate archive file
 IDs.

---
 catalogue/DbConn.cpp                | 15 +++++++++
 catalogue/DbConn.hpp                |  8 +++++
 catalogue/InMemoryCatalogue.cpp     | 49 +++++++++++++++++++++++++++++
 catalogue/InMemoryCatalogue.hpp     | 18 +++++++++++
 catalogue/InMemoryCatalogueTest.cpp | 23 ++++++++------
 catalogue/OracleCatalogue.cpp       |  7 +++++
 catalogue/OracleCatalogue.hpp       | 10 ++++++
 catalogue/RdbmsCatalogue.cpp        |  5 ++-
 catalogue/RdbmsCatalogue.hpp        | 16 ++++++----
 catalogue/SqliteConn.cpp            | 31 +++++++++++-------
 10 files changed, 152 insertions(+), 30 deletions(-)

diff --git a/catalogue/DbConn.cpp b/catalogue/DbConn.cpp
index ee6d700cdb..084348c594 100644
--- a/catalogue/DbConn.cpp
+++ b/catalogue/DbConn.cpp
@@ -17,6 +17,9 @@
  */
 
 #include "catalogue/DbConn.hpp"
+#include "common/exception/Exception.hpp"
+
+#include <memory>
 
 namespace cta {
 namespace catalogue {
@@ -27,5 +30,17 @@ namespace catalogue {
 DbConn::~DbConn() throw() {
 }
 
+//------------------------------------------------------------------------------
+// executeNonQuery
+//------------------------------------------------------------------------------
+void DbConn::executeNonQuery(const std::string &sql) {
+  try {
+    std::unique_ptr<DbStmt> stmt(createStmt(sql));
+    stmt->executeNonQuery();
+  } catch(exception::Exception &ex) {
+    throw exception::Exception(std::string(__FUNCTION__) + " failed: " + ex.what());
+  }
+}
+
 } // namespace catalogue
 } // namespace cta
diff --git a/catalogue/DbConn.hpp b/catalogue/DbConn.hpp
index b666e00f4d..b886783a37 100644
--- a/catalogue/DbConn.hpp
+++ b/catalogue/DbConn.hpp
@@ -47,6 +47,14 @@ public:
    */
   virtual DbStmt *createStmt(const std::string &sql) = 0;
 
+  /**
+   * Convenience function implemented in DbConn around DbConn::createStmt(),
+   * DbStmt::executeNonQuery().
+   *
+   * @sql The SQL statement.
+   */
+  void executeNonQuery(const std::string &sql);
+
   /**
    * Commits the current transaction.
    */
diff --git a/catalogue/InMemoryCatalogue.cpp b/catalogue/InMemoryCatalogue.cpp
index f8bae4c353..4c5b88ac32 100644
--- a/catalogue/InMemoryCatalogue.cpp
+++ b/catalogue/InMemoryCatalogue.cpp
@@ -30,6 +30,23 @@ InMemoryCatalogue::InMemoryCatalogue() {
   std::unique_ptr<SqliteConn> sqliteConn(new SqliteConn(":memory:"));
   m_conn.reset(sqliteConn.release());
   createCatalogueSchema();
+  createArchiveFileIdTable();
+}
+
+//------------------------------------------------------------------------------
+// createArchiveFileIdTable
+//------------------------------------------------------------------------------
+void InMemoryCatalogue::createArchiveFileIdTable() {
+  try {
+    m_conn->executeNonQuery(
+      "CREATE TABLE ARCHIVE_FILE_ID("
+        "ID INTEGER,"
+        "CONSTRAINT ARCHIVE_FILE_ID_PK PRIMARY KEY(ID)"
+      ");");
+    m_conn->executeNonQuery("INSERT INTO ARCHIVE_FILE_ID(ID) VALUES(0);");
+  } catch(std::exception &ex) {
+    throw exception::Exception(std::string(__FUNCTION__) + " failed: " + ex.what());
+  }
 }
 
 //------------------------------------------------------------------------------
@@ -63,5 +80,37 @@ void InMemoryCatalogue::executeNonQueryMultiStmt(const std::string &multiStmt) {
   }
 }
 
+//------------------------------------------------------------------------------
+// getNextArchiveFileId
+//------------------------------------------------------------------------------
+uint64_t InMemoryCatalogue::getNextArchiveFileId() {
+  try {
+    m_conn->executeNonQuery("BEGIN EXCLUSIVE;");
+    m_conn->executeNonQuery("UPDATE ARCHIVE_FILE_ID SET ID = ID + 1;");
+    uint64_t archiveFileId = 0;
+    {
+      const char *const sql =
+        "SELECT "
+          "ID AS ID "
+        "FROM "
+          "ARCHIVE_FILE_ID";
+      std::unique_ptr<DbStmt> stmt(m_conn->createStmt(sql));
+      std::unique_ptr<DbRset> rset(stmt->executeQuery());
+      if(!rset->next()) {
+        throw exception::Exception("ARCHIVE_FILE_ID table is empty");
+      }
+      archiveFileId = rset->columnUint64("ID");
+      if(rset->next()) {
+        throw exception::Exception("Found more than one ID counter in the ARCHIVE_FILE_ID table");
+      }
+    }
+    m_conn->commit();
+
+    return archiveFileId;
+  } catch(exception::Exception &ex) {
+    throw exception::Exception(std::string(__FUNCTION__) + " failed: " + ex.getMessage().str());
+  }
+}
+
 } // namespace catalogue
 } // namespace cta
diff --git a/catalogue/InMemoryCatalogue.hpp b/catalogue/InMemoryCatalogue.hpp
index 9b51e3d42d..5aaefc97a0 100644
--- a/catalogue/InMemoryCatalogue.hpp
+++ b/catalogue/InMemoryCatalogue.hpp
@@ -74,6 +74,24 @@ protected:
    */
   void executeNonQueryMultiStmt(const std::string &multiStmt);
 
+  /**
+   * Returns a unique archive ID that can be used by a new archive file within
+   * the catalogue.
+   *
+   * This method must be implemented by the sub-classes of RdbmsCatalogue
+   * because different database technologies propose different solution to the
+   * problem of generating ever increasing numeric identifiers.
+   */
+  virtual uint64_t getNextArchiveFileId();
+
+private:
+
+  /**
+   * Creates the ARCHIVE_FILE_ID table that will be used to generate ever
+   * incrementing identifiers for archive files.
+   */
+  void createArchiveFileIdTable();
+
 }; // class InMemoryCatalogue
 
 } // namespace catalogue
diff --git a/catalogue/InMemoryCatalogueTest.cpp b/catalogue/InMemoryCatalogueTest.cpp
index 1cb36f6c0a..7711793958 100644
--- a/catalogue/InMemoryCatalogueTest.cpp
+++ b/catalogue/InMemoryCatalogueTest.cpp
@@ -966,16 +966,19 @@ TEST_F(cta_catalogue_InMemoryCatalogueTest, prepareForNewFile) {
   ASSERT_EQ(copyNb, maplet.first);
   ASSERT_EQ(tapePoolName, maplet.second);
 
-  const common::dataStructures::ArchiveFileQueueCriteria queueCriteria =
-    m_catalogue->prepareForNewFile(storageClassName, userIdentity);
-
-  ASSERT_EQ(1, queueCriteria.fileId);
-  ASSERT_EQ(1, queueCriteria.copyToPoolMap.size());
-  ASSERT_EQ(copyNb, queueCriteria.copyToPoolMap.begin()->first);
-  ASSERT_EQ(tapePoolName, queueCriteria.copyToPoolMap.begin()->second);
-  ASSERT_EQ(archivePriority, queueCriteria.mountPolicy.archivePriority);
-  ASSERT_EQ(minArchiveRequestAge, queueCriteria.mountPolicy.archiveMinRequestAge);
-  ASSERT_EQ(maxDrivesAllowed, queueCriteria.mountPolicy.maxDrivesAllowed);
+  for(uint64_t i = 1; i<=10; i++) {
+    const common::dataStructures::ArchiveFileQueueCriteria queueCriteria =
+      m_catalogue->prepareForNewFile(storageClassName, userIdentity);
+
+    ASSERT_EQ(i, queueCriteria.fileId);
+
+    ASSERT_EQ(1, queueCriteria.copyToPoolMap.size());
+    ASSERT_EQ(copyNb, queueCriteria.copyToPoolMap.begin()->first);
+    ASSERT_EQ(tapePoolName, queueCriteria.copyToPoolMap.begin()->second);
+    ASSERT_EQ(archivePriority, queueCriteria.mountPolicy.archivePriority);
+    ASSERT_EQ(minArchiveRequestAge, queueCriteria.mountPolicy.archiveMinRequestAge);
+    ASSERT_EQ(maxDrivesAllowed, queueCriteria.mountPolicy.maxDrivesAllowed);
+  }
 }
 
 TEST_F(cta_catalogue_InMemoryCatalogueTest, prepareToRetrieveFile) {
diff --git a/catalogue/OracleCatalogue.cpp b/catalogue/OracleCatalogue.cpp
index 43dd54bc7b..4db675fce8 100644
--- a/catalogue/OracleCatalogue.cpp
+++ b/catalogue/OracleCatalogue.cpp
@@ -39,5 +39,12 @@ OracleCatalogue::OracleCatalogue(
 OracleCatalogue::~OracleCatalogue() {
 }
 
+//------------------------------------------------------------------------------
+// getNextArchiveFileId
+//------------------------------------------------------------------------------
+uint64_t OracleCatalogue::getNextArchiveFileId() {
+  throw exception::Exception(std::string(__FUNCTION__) + " not implemented");
+}
+
 } // namespace catalogue
 } // namespace cta
diff --git a/catalogue/OracleCatalogue.hpp b/catalogue/OracleCatalogue.hpp
index 54960ae5c9..bb1990c507 100644
--- a/catalogue/OracleCatalogue.hpp
+++ b/catalogue/OracleCatalogue.hpp
@@ -58,6 +58,16 @@ public:
    */
   virtual ~OracleCatalogue();
 
+  /**
+   * Returns a unique archive ID that can be used by a new archive file within
+   * the catalogue.
+   *
+   * This method must be implemented by the sub-classes of RdbmsCatalogue
+   * because different database technologies propose different solution to the
+   * problem of generating ever increasing numeric identifiers.
+   */
+  virtual uint64_t getNextArchiveFileId();
+
 }; // class OracleCatalogue
 
 } // namespace catalogue
diff --git a/catalogue/RdbmsCatalogue.cpp b/catalogue/RdbmsCatalogue.cpp
index dd3351648e..172b89af86 100644
--- a/catalogue/RdbmsCatalogue.cpp
+++ b/catalogue/RdbmsCatalogue.cpp
@@ -33,8 +33,7 @@ namespace catalogue {
 //------------------------------------------------------------------------------
 // constructor
 //------------------------------------------------------------------------------
-RdbmsCatalogue::RdbmsCatalogue():
-  m_nextArchiveFileId(1) {  // This MUST be changed for OCCI - Make SQLite wrapper emulate sequences
+RdbmsCatalogue::RdbmsCatalogue() {
 }
 
 //------------------------------------------------------------------------------
@@ -1946,7 +1945,7 @@ common::dataStructures::ArchiveFileQueueCriteria
 
   // Now that we have both the archive routes and the mount policy it's safe to
   // consume an archive file identifier
-  const uint64_t archiveFileId = m_nextArchiveFileId++;
+  const uint64_t archiveFileId = getNextArchiveFileId();
 
   return common::dataStructures::ArchiveFileQueueCriteria(archiveFileId,
     copyToPoolMap, mountPolicy);
diff --git a/catalogue/RdbmsCatalogue.hpp b/catalogue/RdbmsCatalogue.hpp
index fec8dd5917..6e54f929fa 100644
--- a/catalogue/RdbmsCatalogue.hpp
+++ b/catalogue/RdbmsCatalogue.hpp
@@ -21,7 +21,6 @@
 #include "catalogue/Catalogue.hpp"
 #include "catalogue/DbConn.hpp"
 
-#include <atomic>
 #include <memory>
 #include <mutex>
 
@@ -273,11 +272,6 @@ protected:
    */
   std::unique_ptr<DbConn> m_conn;
 
-  /**
-   * The next unique identifier to be used for an archive file.
-   */
-  std::atomic<uint64_t> m_nextArchiveFileId;
-
   /**
    * Creates the database schema.
    */
@@ -378,6 +372,16 @@ protected:
    */
   std::map<uint64_t, common::dataStructures::TapeFile>getTapeFiles(const uint64_t archiveFileId) const;
 
+  /**
+   * Returns a unique archive ID that can be used by a new archive file within
+   * the catalogue.
+   *
+   * This method must be implemented by the sub-classes of RdbmsCatalogue
+   * because different database technologies propose different solution to the
+   * problem of generating ever increasing numeric identifiers.
+   */
+  virtual uint64_t getNextArchiveFileId() = 0;
+
 }; // class RdbmsCatalogue
 
 } // namespace catalogue
diff --git a/catalogue/SqliteConn.cpp b/catalogue/SqliteConn.cpp
index 31655392a6..54bd117089 100644
--- a/catalogue/SqliteConn.cpp
+++ b/catalogue/SqliteConn.cpp
@@ -66,25 +66,34 @@ void SqliteConn::close() {
 // createStmt
 //------------------------------------------------------------------------------
 DbStmt *SqliteConn::createStmt(const std::string &sql) {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  try {
+    std::lock_guard<std::mutex> lock(m_mutex);
 
-  sqlite3_stmt *stmt = NULL;
-  const int nByte = -1; // Read SQL up to first null terminator
-  const int prepareRc = sqlite3_prepare_v2(m_conn, sql.c_str(), nByte, &stmt, NULL);
-  if(SQLITE_OK != prepareRc) {
-    sqlite3_finalize(stmt);
-    throw exception::Exception(std::string(__FUNCTION__) + " failed for SQL statement " + sql +
-      ": " + sqlite3_errmsg(m_conn));
-  }
+    sqlite3_stmt *stmt = NULL;
+    const int nByte = -1; // Read SQL up to first null terminator
+    const int prepareRc = sqlite3_prepare_v2(m_conn, sql.c_str(), nByte, &stmt, NULL);
+    if (SQLITE_OK != prepareRc) {
+      const std::string msg = sqlite3_errmsg(m_conn);
+      sqlite3_finalize(stmt);
+      throw exception::Exception(msg);
+    }
 
-  return new SqliteStmt(sql, stmt);
+    return new SqliteStmt(sql, stmt);
+  } catch(exception::Exception &ex) {
+    throw exception::Exception(std::string(__FUNCTION__) + " failed for SQL statement " + sql + ": " +
+      ex.getMessage().str());
+  }
 }
 
 //------------------------------------------------------------------------------
 // commit
 //------------------------------------------------------------------------------
 void SqliteConn::commit() {
-  throw exception::Exception(std::string(__FUNCTION__) + " not implemented");
+  try {
+    executeNonQuery("COMMIT;");
+  } catch(exception::Exception &ex) {
+    throw exception::Exception(std::string(__FUNCTION__) + " failed: " + ex.getMessage().str());
+  }
 }
 
 //------------------------------------------------------------------------------
-- 
GitLab