From cc07f50a1d358f8eac3445feb52bfb2fb5c22436 Mon Sep 17 00:00:00 2001 From: Misiu Godfrey <40667992+misiugodfrey@users.noreply.github.com> Date: Thu, 3 Jun 2021 13:49:38 -0700 Subject: [PATCH] Remove leftover storage on table creation * Have initdb -f destroy default disk cache location; also error on table creation if there is pre-cached data in the disk cache for the new table. --- Catalog/Catalog.cpp | 8 ++++++++ Tests/CommandLineTest.cpp | 17 +++++++++++++++++ Tests/ForeignTableDmlTest.cpp | 1 + initdb.cpp | 10 ++++++++++ 4 files changed, 36 insertions(+) diff --git a/Catalog/Catalog.cpp b/Catalog/Catalog.cpp index f88b1b0a13..9b3ce17c12 100644 --- a/Catalog/Catalog.cpp +++ b/Catalog/Catalog.cpp @@ -2399,6 +2399,14 @@ void Catalog::createTable( } try { + auto cache = dataMgr_->getPersistentStorageMgr()->getDiskCache(); + if (cache) { + CHECK(!cache->hasCachedMetadataForKeyPrefix({getCurrentDB().dbId, td.tableId})) + << "Disk cache at " + cache->getCacheDirectory() + << " contains preexisting data for new table. Please " + "delete or clear cache before continuing"; + } + addTableToMap(&td, cds, dds); calciteMgr_->updateMetadata(currentDB_.dbName, td.tableName); if (!td.storageType.empty() && td.storageType != StorageType::FOREIGN_TABLE) { diff --git a/Tests/CommandLineTest.cpp b/Tests/CommandLineTest.cpp index 64812b488c..0723206032 100644 --- a/Tests/CommandLineTest.cpp +++ b/Tests/CommandLineTest.cpp @@ -170,11 +170,28 @@ TEST_F(InitDBTest, AlreadyInit) { "OmniSci catalogs already initialized at " + get_temp_dir() + ". Use -f to force reinitialization."); } +// Blocked by existing cache. +TEST_F(InitDBTest, ExistingCache) { + boost::filesystem::create_directory(get_temp_dir() + "/omnisci_disk_cache"); + CommandLineTestcase(get_executable(), + get_temp_dir(), + 1, + "", + "OmniSci disk cache already exists at " + get_temp_dir() + + "/omnisci_disk_cache" + ". Use -f to force reinitialization."); +} // Override existing database. TEST_F(InitDBTest, Force) { CommandLineTestcase(get_executable(), get_temp_dir(), 0, "", ""); CommandLineTestcase(get_executable(), get_temp_dir() + " -f", 0, "", ""); } +// Override existing cache +TEST_F(InitDBTest, ForceCache) { + boost::filesystem::create_directory(get_temp_dir() + "/omnisci_disk_cache"); + boost::filesystem::create_directory(get_temp_dir() + "/omnisci_disk_cache/temp"); + CommandLineTestcase(get_executable(), get_temp_dir() + " -f", 0, "", ""); + ASSERT_FALSE(boost::filesystem::exists(get_temp_dir() + "/omnisci_disk_cache/temp")); +} // Data directory does not exist. TEST_F(InitDBTest, MissingDir) { CommandLineTestcase(get_executable(), diff --git a/Tests/ForeignTableDmlTest.cpp b/Tests/ForeignTableDmlTest.cpp index da3b46879b..3db93ca5a1 100644 --- a/Tests/ForeignTableDmlTest.cpp +++ b/Tests/ForeignTableDmlTest.cpp @@ -26,6 +26,7 @@ #include "DBHandlerTestHelpers.h" #include "DataMgr/ForeignStorage/ForeignStorageCache.h" #include "DataMgr/ForeignStorage/ForeignTableRefresh.h" +#include "DataMgrTestHelpers.h" #include "Geospatial/Types.h" #include "ImportExport/DelimitedParserUtils.h" #include "TestHelpers.h" diff --git a/initdb.cpp b/initdb.cpp index 55e14a3f00..748b584b15 100644 --- a/initdb.cpp +++ b/initdb.cpp @@ -129,6 +129,16 @@ int main(int argc, char* argv[]) { return 1; } } + std::string disk_cache_path = base_path + "/omnisci_disk_cache"; + if (boost::filesystem::exists(disk_cache_path)) { + if (force) { + boost::filesystem::remove_all(disk_cache_path); + } else { + std::cerr << "OmniSci disk cache already exists at " + disk_cache_path + + ". Use -f to force reinitialization.\n"; + return 1; + } + } if (!boost::filesystem::create_directory(catalogs_path)) { std::cerr << "Cannot create mapd_catalogs subdirectory under " << base_path << std::endl;