8000 make database deletion more deterministic · sleepycat/arangodb@0156c5a · GitHub
[go: up one dir, main page]
Skip to content

Commit 0156c5a

Browse files
jsteemannjsteemann
committed
make database deletion more deterministic
1 parent 7939bd7 commit 0156c5a

File tree

5 files changed

+173
-32
lines changed

5 files changed

+173
-32
lines changed

‎arangod/MMFiles/MMFilesEngine.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ void MMFilesEngine::getDatabases(arangodb::velocypack::Builder& result) {
313313
if (!idSlice.isString() ||
314314
id != static_cast<TRI_voc_tick_t>(basics::StringUtils::uint64(idSlice.copyString()))) {
315315
LOG(ERR) << "database directory '" << directory
316-
<< "' does not contain a valid parameters file";
316+
<< "' does not contain a valid parameters file. database id is not a string";
317317
THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_ILLEGAL_PARAMETER_FILE);
318318
}
319319

@@ -1237,7 +1237,14 @@ TRI_vocbase_t* MMFilesEngine::openExistingDatabase(TRI_voc_tick_t id, std::strin
12371237

12381238
/// @brief physically erases the database directory
12391239
int MMFilesEngine::dropDatabaseDirectory(std::string const& path) {
1240-
return TRI_RemoveDirectory(path.c_str());
1240+
// first create a .tmp file in the directory that will help us recover when we crash
1241+
// before the directory deletion is completed
1242+
std::string const tmpfile(
1243+
arangodb::basics::FileUtils::buildFilename(path, ".tmp"));
1244+
// ignore errors from writing this file...
1245+
TRI_WriteFile(tmpfile.c_str(), "", 0);
1246+
1247+
return TRI_RemoveDirectoryDeterministic(path.c_str());
12411248
}
12421249

12431250
/// @brief iterate over a set of datafiles, identified by filenames

‎js/client/modules/@arangodb/testing.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3250,6 +3250,7 @@ const recoveryTests = [
32503250
'empty-datafiles',
32513251
'flush-drop-database-and-fail',
32523252
'drop-database-flush-and-fail',
3253+
'drop-database-only-tmp',
32533254
'create-databases',
32543255
'recreate-databases',
32553256
'drop-databases',

‎js/server/tests/recovery/drop-database-only-tmp.js

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/* jshint globalstrict:false, strict:false, unused: false */
2+
/* global fail, assertEqual */
3+
// //////////////////////////////////////////////////////////////////////////////
4+
// / @brief tests for transactions
5+
// /
6+
// / @file
7+
// /
8+
// / DISCLAIMER
9+
// /
10+
// / Copyright 2010-2012 triagens GmbH, Cologne, Germany
11+
// /
12+
// / Licensed under the Apache License, Version 2.0 (the "License")
13+
// / you may not use this file except in compliance with the License.
14+
// / You may obtain a copy of the License at
15+
// /
16+
// / http://www.apache.org/licenses/LICENSE-2.0
17+
// /
18+
// / Unless required by applicable law or agreed to in writing, software
19+
// / distributed under the License is distributed on an "AS IS" BASIS,
20+
// / WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
21+
// / See the License for the specific language governing permissions and
22+
// / limitations under the License.
23+
// /
24+
// / Copyright holder is triAGENS GmbH, Cologne, Germany
25+
// /
26+
// / @author Jan Steemann
27+
// / @author Copyright 2013, triAGENS GmbH, Cologne, Germany
28+
// //////////////////////////////////////////////////////////////////////////////
29+
30+
var db = require('@arangodb').db;
31+
var internal = require('internal');
32+
var fs = require('fs');
33+
var jsunity = require('jsunity');
34+
35+
function runSetup () {
36+
'use strict';
37+
internal.debugClearFailAt();
38+
39+
var path = fs.join(fs.join(db._path(), ".."), "database-999999999");
40+
fs.makeDirectory(path);
41+
42+
var data = { deleted: true, name: "UnitTestsRecovery", id: "999999999" };
43+
fs.writeFile(fs.join(path, "parameter.json"), JSON.stringify(data));
44+
fs.writeFile(fs.join(path, ".tmp"), "");
45+
46+
internal.debugSegfault('crashing server');
47+
}
48+
49+
// //////////////////////////////////////////////////////////////////////////////
50+
// / @brief test suite
51+
// //////////////////////////////////////////////////////////////////////////////
52+
53+
function recoverySuite () {
54+
'use strict';
55+
jsunity.jsUnity.attachAssertions();
56+
57+
return {
58+
setUp: function () {},
59+
tearDown: function () {},
60+
61+
// //////////////////////////////////////////////////////////////////////////////
62+
// / @brief test whether we the data are correct after restart
63+
// //////////////////////////////////////////////////////////////////////////////
64+
65+
testDropDatabaseOnlyTmp: function () {
66+
try {
67+
db._useDatabase('UnitTestsRecovery');
68+
fail();
69+
} catch (err) {
70+
assertEqual(internal.errors.ERROR_ARANGO_DATABASE_NOT_FOUND.code, err.errorNum);
71+
}
72+
}
73+
74+
};
75+
}
76+
77+
// //////////////////////////////////////////////////////////////////////////////
78+
// / @brief executes the test suite
79+
// //////////////////////////////////////////////////////////////////////////////
80+
81+
function main (argv) {
82+
'use strict';
83+
if (argv[1] === 'setup') {
84+
runSetup();
85+
return 0;
86+
} else {
87+
jsunity.run(recoverySuite);
88+
return jsunity.done().status ? 0 : 1;
89+
}
90+
}

‎lib/Basics/files.cpp

Lines changed: 65 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -575,19 +575,14 @@ int TRI_RemoveDirectory(char const* filename) {
575575
LOG(TRACE) << "removing symbolic link '" << filename << "'";
576576
return TRI_UnlinkFile(filename);
577577
} else if (TRI_IsDirectory(filename)) {
578-
int res;
579-
580578
LOG(TRACE) << "removing directory '" << filename << "'";
581579

582-
res = TRI_ERROR_NO_ERROR;
580+
int res = TRI_ERROR_NO_ERROR;
583581
std::vector<std::string> files = TRI_FilesDirectory(filename);
584582
for (auto const& dir : files) {
585-
char* full;
586-
int subres;
587-
588-
full = TRI_Concatenate2File(filename, dir.c_str());
583+
char* full = TRI_Concatenate2File(filename, dir.c_str());
589584

590-
subres = TRI_RemoveDirectory(full);
585+
int subres = TRI_RemoveDirectory(full);
591586
TRI_FreeString(TRI_CORE_MEM_ZONE, full);
592587

593588
if (subres != TRI_ERROR_NO_ERROR) {
@@ -608,21 +603,60 @@ int TRI_RemoveDirectory(char const* filename) {
608603
LOG(TRACE) << "attempt to remove non-existing file/directory '" << filename
609604
<< "'";
610605

606+
// TODO: why do we actually return "no error" here?
611607
return TRI_ERROR_NO_ERROR;
612608
}
613609
}
614610

611+
////////////////////////////////////////////////////////////////////////////////
612+
/// @brief removes a directory recursively
613+
////////////////////////////////////////////////////////////////////////////////
614+
615+
int TRI_RemoveDirectoryDeterministic(char const* filename) {
616+
std::vector<std::string> files = TRI_FullTreeDirectory(filename);
617+
// start removing files from long names to short names
618+
std::sort(files.begin(), files.end(), [](std::string const& lhs, std::string const& rhs) -> bool {
619+
if (lhs.size() == rhs.size()) {
620+
// equal lengths. now compare the file/directory names
621+
return lhs < rhs;
622+
}
623+
return lhs.size() > rhs.size();
624+
});
625+
626+
// LOG(TRACE) << "removing files in directory '" << filename << "' in this order: " << files;
627+
628+
int res = TRI_ERROR_NO_ERROR;
629+
630+
for (auto const& it : files) {
631+
if (it.empty()) {
632+
// TRI_FullTreeDirectory returns "" as its first member
633+
continue;
634+
}
635+
636+
char* full = TRI_Concatenate2File(filename, it.c_str());
637+
int subres = TRI_RemoveDirectory(full);
638+
TRI_FreeString(TRI_CORE_MEM_ZONE, full);
639+
640+
if (subres != TRI_ERROR_NO_ERROR) {
641+
res = subres;
642+
}
643+
}
644+
645+
int subres = TRI_RemoveDirectory(filename);
646+
if (subres != TRI_ERROR_NO_ERROR) {
647+
res = subres;
648+
}
649+
650+
return res;
651+
}
652+
615653
////////////////////////////////////////////////////////////////////////////////
616654
/// @brief extracts the dirname
617655
////////////////////////////////////////////////////////////////////////////////
618656

619657
char* TRI_Dirname(char const* path) {
620-
size_t n;
621-
size_t m;
622-
char const* p;
623-
624-
n = strlen(path);
625-
m = 0;
658+
size_t n = strlen(path);
659+
size_t m = 0;
626660

627661
if (1 < n) {
628662
if (path[n - 1] == TRI_DIR_SEPARATOR_CHAR) {
@@ -640,6 +674,7 @@ char* TRI_Dirname(char const* path) {
640674
return TRI_DuplicateString("..");
641675
}
642676

677+
char const* p;
643678
for (p = path + (n - m - 1); path < p; --p) {
644679
if (*p == TRI_DIR_SEPARATOR_CHAR) {
645680
break;
@@ -782,28 +817,29 @@ std::vector<std::string> TRI_FilesDirectory(char const* path) {
782817
std::vector<std::string> TRI_FilesDirectory(char const* path) {
783818
std::vector<std::string> result;
784819

785-
DIR* d;
786-
struct dirent* de;
787-
788-
d = opendir(path);
820+
DIR* d = opendir(path);
789821

790-
if (d == 0) {
822+
if (d == nullptr) {
791823
return result;
792824
}
793825

794-
de = readdir(d);
826+
struct dirent* de = readdir(d);
795827

796-
while (de != 0) {
797-
if (strcmp(de->d_name, ".") != 0 && strcmp(de->d_name, "..") != 0) {
798-
result.emplace_back(de->d_name);
799-
}
828+
try {
829+
while (de != nullptr) {
830+
if (strcmp(de->d_name, ".") != 0 && strcmp(de->d_name, "..") != 0) {
831+
// may throw
832+
result.emplace_back(std::string(de->d_name));
833+
}
800834

801-
de = readdir(d);
835+
de = readdir(d);
836+
}
837+
closedir(d);
838+
return result;
839+
} catch (...) {
840+
closedir(d);
841+
throw;
802842
}
803-
804-
closedir(d);
805-
806-
return result;
807843
}
808844

809845
#endif

‎lib/Basics/files.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,18 @@ int TRI_CreateDirectory(char const* path, long& systemError,
101101
int TRI_RemoveEmptyDirectory(char const* filename);
102102

103103
////////////////////////////////////////////////////////////////////////////////
104-
/// @brief removes a directory recursively
104+
/// @brief removes a directory recursively, using file order provided by
105+
/// the file system
105106
////////////////////////////////////////////////////////////////////////////////
106107

107108
int TRI_RemoveDirectory(char const* filename);
108109

110+
////////////////////////////////////////////////////////////////////////////////
111+
/// @brief removes a directory recursively, using a deterministic order of files
112+
////////////////////////////////////////////////////////////////////////////////
113+
114+
int TRI_RemoveDirectoryDeterministic(char const* filename);
115+
109116
////////////////////////////////////////////////////////////////////////////////
110117
/// @brief extracts the dirname
111118
////////////////////////////////////////////////////////////////////////////////

0 commit comments

Comments
 (0)
0