Skip to content

Commit 321bb91

Browse files
MahatiCMahatiC
committed
remove empty db hash and suffix directories
If a db gets quarantined we may fail to cleanup an empty suffix dir or a hash dir. Change-Id: I721fa5fe9a7ae22eead8d5141f93e116847ca058 Closes-Bug: #1583719
1 parent d792032 commit 321bb91

File tree

2 files changed

+158
-0
lines changed

2 files changed

+158
-0
lines changed

‎swift/common/db_replicator.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,18 +89,31 @@ def walk_datadir(datadir, node_id):
8989
suffixes = os.listdir(part_dir)
9090
if not suffixes:
9191
os.rmdir(part_dir)
92+
continue
9293
for suffix in suffixes:
9394
suff_dir = os.path.join(part_dir, suffix)
9495
if not os.path.isdir(suff_dir):
9596
continue
9697
hashes = os.listdir(suff_dir)
98+
if not hashes:
99+
os.rmdir(suff_dir)
100+
continue
97101
for hsh in hashes:
98102
hash_dir = os.path.join(suff_dir, hsh)
99103
if not os.path.isdir(hash_dir):
100104
continue
101105
object_file = os.path.join(hash_dir, hsh + '.db')
102106
if os.path.exists(object_file):
103107
yield (partition, object_file, node_id)
108+
else:
109+
try:
110+
os.rmdir(hash_dir)
111+
except OSError as e:
112+
if e.errno is not errno.ENOTEMPTY:
113+
raise
114+
# remove empty partitions after the above directory walk
115+
if not suffixes:
116+
os.rmdir(part_dir)
104117

105118
its = [walk_datadir(datadir, node_id) for datadir, node_id in datadirs]
106119
while its:

‎test/unit/common/test_db_replicator.py

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
from test import unit
3838
from test.unit.common.test_db import ExampleBroker
39+
from test.unit import with_tempdir
3940

4041

4142
TEST_ACCOUNT_NAME = 'a c t'
@@ -1048,6 +1049,150 @@ def test_complete_rsync(self):
10481049
finally:
10491050
rmtree(drive)
10501051

1052+
@with_tempdir
1053+
def test_empty_suffix_and_hash_dirs_get_cleanedup(self, tempdir):
1054+
# tests empty suffix and hash dirs of a quarantined db are cleaned up
1055+
listdir_calls = []
1056+
isdir_calls = []
1057+
exists_calls = []
1058+
shuffle_calls = []
1059+
rmdir_calls = []
1060+
1061+
existing_file = os.path.join(tempdir, 'a_file_exists')
1062+
open(existing_file, 'a').close()
1063+
dir_with_no_obj_file = '/srv/node/sdb/containers/9876/xyz/' \
1064+
'11111111111111111111111111111xyz'
1065+
1066+
def _listdir(path):
1067+
listdir_calls.append(path)
1068+
if not path.startswith('/srv/node/sda/containers') and \
1069+
not path.startswith('/srv/node/sdb/containers'):
1070+
return []
1071+
path = path[len('/srv/node/sdx/containers'):]
1072+
if path == '':
1073+
return ['6789', '9876']
1074+
elif path == '/9876':
1075+
return ['xyz']
1076+
elif path == '/9876/xyz':
1077+
return ['11111111111111111111111111111xyz']
1078+
elif path == '/9876/xyz/11111111111111111111111111111xyz':
1079+
return []
1080+
elif path == '/6789':
1081+
return ['jkl']
1082+
elif path == '/6789/jkl':
1083+
return []
1084+
return []
1085+
1086+
def _isdir(path):
1087+
isdir_calls.append(path)
1088+
if not path.startswith('/srv/node/sda/containers') and \
1089+
not path.startswith('/srv/node/sdb/containers'):
1090+
return False
1091+
path = path[len('/srv/node/sdx/containers'):]
1092+
if path in ('/9876', '/9876/xyz',
1093+
'/9876/xyz/11111111111111111111111111111xyz',
1094+
'/6789', '/6789/jkl'):
1095+
return True
1096+
return False
1097+
1098+
def _exists(arg):
1099+
exists_calls.append(arg)
1100+
if arg in ('/srv/node/sda/containers/9876/xyz/'
1101+
'11111111111111111111111111111xyz/'
1102+
'11111111111111111111111111111xyz.db',
1103+
'/srv/node/sdb/containers/9876/xyz/'
1104+
'11111111111111111111111111111xyz/'
1105+
'11111111111111111111111111111xyz.db'):
1106+
return False
1107+
return True
1108+
1109+
def _shuffle(arg):
1110+
shuffle_calls.append(arg)
1111+
1112+
def _rmdir(arg):
1113+
rmdir_calls.append(arg)
1114+
if arg == dir_with_no_obj_file:
1115+
# use db_replicator.os.rmdir to delete a directory with an
1116+
# existing file; fail if it doesn't handle OSError
1117+
orig_rmdir(tempdir)
1118+
self.fail('The rmdir did not handle the exception as expected')
1119+
1120+
orig_listdir = db_replicator.os.listdir
1121+
orig_isdir = db_replicator.os.path.isdir
1122+
orig_exists = db_replicator.os.path.exists
1123+
orig_shuffle = db_replicator.random.shuffle
1124+
orig_rmdir = db_replicator.os.rmdir
1125+
1126+
try:
1127+
db_replicator.os.listdir = _listdir
1128+
db_replicator.os.path.isdir = _isdir
1129+
db_replicator.os.path.exists = _exists
1130+
db_replicator.random.shuffle = _shuffle
1131+
db_replicator.os.rmdir = _rmdir
1132+
1133+
datadirs = [('/srv/node/sda/containers', 1),
1134+
('/srv/node/sdb/containers', 2)]
1135+
results = list(db_replicator.roundrobin_datadirs(datadirs))
1136+
# The results show that there are no .db files to be returned
1137+
# in this case
1138+
self.assertEqual(results, [])
1139+
# The listdir calls show that we only listdir the dirs
1140+
self.assertEqual(listdir_calls, [
1141+
'/srv/node/sda/containers',
1142+
'/srv/node/sda/containers/6789',
1143+
'/srv/node/sda/containers/6789/jkl',
1144+
'/srv/node/sda/containers/9876',
1145+
'/srv/node/sda/containers/9876/xyz',
1146+
'/srv/node/sdb/containers',
1147+
'/srv/node/sdb/containers/6789',
1148+
'/srv/node/sdb/containers/6789/jkl',
1149+
'/srv/node/sdb/containers/9876',
1150+
'/srv/node/sdb/containers/9876/xyz'])
1151+
# The isdir calls show that we did ask about the things pretending
1152+
# to be files at various levels.
1153+
self.assertEqual(isdir_calls, [
1154+
'/srv/node/sda/containers/6789',
1155+
'/srv/node/sda/containers/6789/jkl',
1156+
'/srv/node/sda/containers/9876',
1157+
'/srv/node/sda/containers/9876/xyz',
1158+
('/srv/node/sda/containers/9876/xyz/'
1159+
'11111111111111111111111111111xyz'),
1160+
'/srv/node/sdb/containers/6789',
1161+
'/srv/node/sdb/containers/6789/jkl',
1162+
'/srv/node/sdb/containers/9876',
1163+
'/srv/node/sdb/containers/9876/xyz',
1164+
('/srv/node/sdb/containers/9876/xyz/'
1165+
'11111111111111111111111111111xyz')])
1166+
# The exists calls are the .db files we looked for as we walked the
1167+
# structure.
1168+
self.assertEqual(exists_calls, [
1169+
('/srv/node/sda/containers/9876/xyz/'
1170+
'11111111111111111111111111111xyz/'
1171+
'11111111111111111111111111111xyz.db'),
1172+
('/srv/node/sdb/containers/9876/xyz/'
1173+
'11111111111111111111111111111xyz/'
1174+
'11111111111111111111111111111xyz.db')])
1175+
# Shows that we called shuffle twice, once for each device.
1176+
self.assertEqual(
1177+
shuffle_calls, [['6789', '9876'],
1178+
['6789', '9876']])
1179+
1180+
# Shows that we called rmdir and removed directories with no db
1181+
# files and directories with no hashes
1182+
self.assertEqual(
1183+
rmdir_calls, ['/srv/node/sda/containers/6789/jkl',
1184+
'/srv/node/sda/containers/9876/xyz/'
1185+
'11111111111111111111111111111xyz',
1186+
'/srv/node/sdb/containers/6789/jkl',
1187+
'/srv/node/sdb/containers/9876/xyz/'
1188+
'11111111111111111111111111111xyz'])
1189+
finally:
1190+
db_replicator.os.listdir = orig_listdir
1191+
db_replicator.os.path.isdir = orig_isdir
1192+
db_replicator.os.path.exists = orig_exists
1193+
db_replicator.random.shuffle = orig_shuffle
1194+
db_replicator.os.rmdir = orig_rmdir
1195+
10511196
def test_roundrobin_datadirs(self):
10521197
listdir_calls = []
10531198
isdir_calls = []

0 commit comments

Comments
 (0)