diff --git a/arangod/MMFiles/MMFilesEngine.cpp b/arangod/MMFiles/MMFilesEngine.cpp index 9116b06eb7..cc516352a6 100644 --- a/arangod/MMFiles/MMFilesEngine.cpp +++ b/arangod/MMFiles/MMFilesEngine.cpp @@ -313,7 +313,7 @@ void MMFilesEngine::getDatabases(arangodb::velocypack::Builder& result) { if (!idSlice.isString() || id != static_cast(basics::StringUtils::uint64(idSlice.copyString()))) { LOG(ERR) << "database directory '" << directory - << "' does not contain a valid parameters file"; + << "' does not contain a valid parameters file. database id is not a string"; THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_ILLEGAL_PARAMETER_FILE); } @@ -1237,7 +1237,14 @@ TRI_vocbase_t* MMFilesEngine::openExistingDatabase(TRI_voc_tick_t id, std::strin /// @brief physically erases the database directory int MMFilesEngine::dropDatabaseDirectory(std::string const& path) { - return TRI_RemoveDirectory(path.c_str()); + // first create a .tmp file in the directory that will help us recover when we crash + // before the directory deletion is completed + std::string const tmpfile( + arangodb::basics::FileUtils::buildFilename(path, ".tmp")); + // ignore errors from writing this file... + TRI_WriteFile(tmpfile.c_str(), "", 0); + + return TRI_RemoveDirectoryDeterministic(path.c_str()); } /// @brief iterate over a set of datafiles, identified by filenames diff --git a/js/client/modules/@arangodb/testing.js b/js/client/modules/@arangodb/testing.js index 2225ecdb8c..533f434378 100644 --- a/js/client/modules/@arangodb/testing.js +++ b/js/client/modules/@arangodb/testing.js @@ -3250,6 +3250,7 @@ const recoveryTests = [ 'empty-datafiles', 'flush-drop-database-and-fail', 'drop-database-flush-and-fail', + 'drop-database-only-tmp', 'create-databases', 'recreate-databases', 'drop-databases', diff --git a/js/server/tests/recovery/drop-database-only-tmp.js b/js/server/tests/recovery/drop-database-only-tmp.js new file mode 100644 index 0000000000..80180d5f04 --- /dev/null +++ b/js/server/tests/recovery/drop-database-only-tmp.js @@ -0,0 +1,90 @@ +/* jshint globalstrict:false, strict:false, unused: false */ +/* global fail, assertEqual */ +// ////////////////////////////////////////////////////////////////////////////// +// / @brief tests for transactions +// / +// / @file +// / +// / DISCLAIMER +// / +// / Copyright 2010-2012 triagens GmbH, Cologne, Germany +// / +// / Licensed under the Apache License, Version 2.0 (the "License") +// / you may not use this file except in compliance with the License. +// / You may obtain a copy of the License at +// / +// / http://www.apache.org/licenses/LICENSE-2.0 +// / +// / Unless required by applicable law or agreed to in writing, software +// / distributed under the License is distributed on an "AS IS" BASIS, +// / WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// / See the License for the specific language governing permissions and +// / limitations under the License. +// / +// / Copyright holder is triAGENS GmbH, Cologne, Germany +// / +// / @author Jan Steemann +// / @author Copyright 2013, triAGENS GmbH, Cologne, Germany +// ////////////////////////////////////////////////////////////////////////////// + +var db = require('@arangodb').db; +var internal = require('internal'); +var fs = require('fs'); +var jsunity = require('jsunity'); + +function runSetup () { + 'use strict'; + internal.debugClearFailAt(); + + var path = fs.join(fs.join(db._path(), ".."), "database-999999999"); + fs.makeDirectory(path); + + var data = { deleted: true, name: "UnitTestsRecovery", id: "999999999" }; + fs.writeFile(fs.join(path, "parameter.json"), JSON.stringify(data)); + fs.writeFile(fs.join(path, ".tmp"), ""); + + internal.debugSegfault('crashing server'); +} + +// ////////////////////////////////////////////////////////////////////////////// +// / @brief test suite +// ////////////////////////////////////////////////////////////////////////////// + +function recoverySuite () { + 'use strict'; + jsunity.jsUnity.attachAssertions(); + + return { + setUp: function () {}, + tearDown: function () {}, + + // ////////////////////////////////////////////////////////////////////////////// + // / @brief test whether we the data are correct after restart + // ////////////////////////////////////////////////////////////////////////////// + + testDropDatabaseOnlyTmp: function () { + try { + db._useDatabase('UnitTestsRecovery'); + fail(); + } catch (err) { + assertEqual(internal.errors.ERROR_ARANGO_DATABASE_NOT_FOUND.code, err.errorNum); + } + } + + }; +} + +// ////////////////////////////////////////////////////////////////////////////// +// / @brief executes the test suite +// ////////////////////////////////////////////////////////////////////////////// + +function main (argv) { + 'use strict'; + if (argv[1] === 'setup') { + runSetup(); + return 0; + } else { + jsunity.run(recoverySuite); + return jsunity.done().status ? 0 : 1; + } +} diff --git a/lib/Basics/files.cpp b/lib/Basics/files.cpp index bb908d5621..c98ae1e986 100644 --- a/lib/Basics/files.cpp +++ b/lib/Basics/files.cpp @@ -575,19 +575,14 @@ int TRI_RemoveDirectory(char const* filename) { LOG(TRACE) << "removing symbolic link '" << filename << "'"; return TRI_UnlinkFile(filename); } else if (TRI_IsDirectory(filename)) { - int res; - LOG(TRACE) << "removing directory '" << filename << "'"; - res = TRI_ERROR_NO_ERROR; + int res = TRI_ERROR_NO_ERROR; std::vector files = TRI_FilesDirectory(filename); for (auto const& dir : files) { - char* full; - int subres; + char* full = TRI_Concatenate2File(filename, dir.c_str()); - full = TRI_Concatenate2File(filename, dir.c_str()); - - subres = TRI_RemoveDirectory(full); + int subres = TRI_RemoveDirectory(full); TRI_FreeString(TRI_CORE_MEM_ZONE, full); if (subres != TRI_ERROR_NO_ERROR) { @@ -608,21 +603,60 @@ int TRI_RemoveDirectory(char const* filename) { LOG(TRACE) << "attempt to remove non-existing file/directory '" << filename << "'"; + // TODO: why do we actually return "no error" here? return TRI_ERROR_NO_ERROR; } } +//////////////////////////////////////////////////////////////////////////////// +/// @brief removes a directory recursively +//////////////////////////////////////////////////////////////////////////////// + +int TRI_RemoveDirectoryDeterministic(char const* filename) { + std::vector files = TRI_FullTreeDirectory(filename); + // start removing files from long names to short names + std::sort(files.begin(), files.end(), [](std::string const& lhs, std::string const& rhs) -> bool { + if (lhs.size() == rhs.size()) { + // equal lengths. now compare the file/directory names + return lhs < rhs; + } + return lhs.size() > rhs.size(); + }); + + // LOG(TRACE) << "removing files in directory '" << filename << "' in this order: " << files; + + int res = TRI_ERROR_NO_ERROR; + + for (auto const& it : files) { + if (it.empty()) { + // TRI_FullTreeDirectory returns "" as its first member + continue; + } + + char* full = TRI_Concatenate2File(filename, it.c_str()); + int subres = TRI_RemoveDirectory(full); + TRI_FreeString(TRI_CORE_MEM_ZONE, full); + + if (subres != TRI_ERROR_NO_ERROR) { + res = subres; + } + } + + int subres = TRI_RemoveDirectory(filename); + if (subres != TRI_ERROR_NO_ERROR) { + res = subres; + } + + return res; +} + //////////////////////////////////////////////////////////////////////////////// /// @brief extracts the dirname //////////////////////////////////////////////////////////////////////////////// char* TRI_Dirname(char const* path) { - size_t n; - size_t m; - char const* p; - - n = strlen(path); - m = 0; + size_t n = strlen(path); + size_t m = 0; if (1 < n) { if (path[n - 1] == TRI_DIR_SEPARATOR_CHAR) { @@ -640,6 +674,7 @@ char* TRI_Dirname(char const* path) { return TRI_DuplicateString(".."); } + char const* p; for (p = path + (n - m - 1); path < p; --p) { if (*p == TRI_DIR_SEPARATOR_CHAR) { break; @@ -782,28 +817,29 @@ std::vector TRI_FilesDirectory(char const* path) { std::vector TRI_FilesDirectory(char const* path) { std::vector result; - DIR* d; - struct dirent* de; + DIR* d = opendir(path); - d = opendir(path); - - if (d == 0) { + if (d == nullptr) { return result; } - de = readdir(d); + struct dirent* de = readdir(d); - while (de != 0) { - if (strcmp(de->d_name, ".") != 0 && strcmp(de->d_name, "..") != 0) { - result.emplace_back(de->d_name); + try { + while (de != nullptr) { + if (strcmp(de->d_name, ".") != 0 && strcmp(de->d_name, "..") != 0) { + // may throw + result.emplace_back(std::string(de->d_name)); + } + + de = readdir(d); } - - de = readdir(d); + closedir(d); + return result; + } catch (...) { + closedir(d); + throw; } - - closedir(d); - - return result; } #endif diff --git a/lib/Basics/files.h b/lib/Basics/files.h index a41ed674ed..2b96f7fec1 100644 --- a/lib/Basics/files.h +++ b/lib/Basics/files.h @@ -101,11 +101,18 @@ int TRI_CreateDirectory(char const* path, long& systemError, int TRI_RemoveEmptyDirectory(char const* filename); //////////////////////////////////////////////////////////////////////////////// -/// @brief removes a directory recursively +/// @brief removes a directory recursively, using file order provided by +/// the file system //////////////////////////////////////////////////////////////////////////////// int TRI_RemoveDirectory(char const* filename); +//////////////////////////////////////////////////////////////////////////////// +/// @brief removes a directory recursively, using a deterministic order of files +//////////////////////////////////////////////////////////////////////////////// + +int TRI_RemoveDirectoryDeterministic(char const* filename); + //////////////////////////////////////////////////////////////////////////////// /// @brief extracts the dirname ////////////////////////////////////////////////////////////////////////////////