diff --git a/swift/common/db.py b/swift/common/db.py index ce4def9d02..048d5e308c 100644 --- a/swift/common/db.py +++ b/swift/common/db.py @@ -322,25 +322,11 @@ class DatabaseBroker(object): self._delete_db(conn, timestamp) conn.commit() - def possibly_quarantine(self, exc_type, exc_value, exc_traceback): + def quarantine(self, reason): """ - Checks the exception info to see if it indicates a quarantine situation - (malformed or corrupted database). If not, the original exception will - be reraised. If so, the database will be quarantined and a new + The database will be quarantined and a sqlite3.DatabaseError will be raised indicating the action taken. """ - if 'database disk image is malformed' in str(exc_value): - exc_hint = 'malformed' - elif 'malformed database schema' in str(exc_value): - exc_hint = 'malformed' - elif ' is not a database' in str(exc_value): - # older versions said 'file is not a database' - # now 'file is encrypted or is not a database' - exc_hint = 'corrupted' - elif 'disk I/O error' in str(exc_value): - exc_hint = 'disk error while accessing' - else: - six.reraise(exc_type, exc_value, exc_traceback) prefix_path = os.path.dirname(self.db_dir) partition_path = os.path.dirname(prefix_path) dbs_path = os.path.dirname(partition_path) @@ -356,12 +342,34 @@ class DatabaseBroker(object): quar_path = "%s-%s" % (quar_path, uuid4().hex) renamer(self.db_dir, quar_path, fsync=False) detail = _('Quarantined %(db_dir)s to %(quar_path)s due to ' - '%(exc_hint)s database') % {'db_dir': self.db_dir, - 'quar_path': quar_path, - 'exc_hint': exc_hint} + '%(reason)s') % {'db_dir': self.db_dir, + 'quar_path': quar_path, + 'reason': reason} self.logger.error(detail) raise sqlite3.DatabaseError(detail) + def possibly_quarantine(self, exc_type, exc_value, exc_traceback): + """ + Checks the exception info to see if it indicates a quarantine situation + (malformed or corrupted database). If not, the original exception will + be reraised. If so, the database will be quarantined and a new + sqlite3.DatabaseError will be raised indicating the action taken. + """ + if 'database disk image is malformed' in str(exc_value): + exc_hint = 'malformed database' + elif 'malformed database schema' in str(exc_value): + exc_hint = 'malformed database' + elif ' is not a database' in str(exc_value): + # older versions said 'file is not a database' + # now 'file is encrypted or is not a database' + exc_hint = 'corrupted database' + elif 'disk I/O error' in str(exc_value): + exc_hint = 'disk error while accessing database' + else: + six.reraise(exc_type, exc_value, exc_traceback) + + self.quarantine(exc_hint) + @contextmanager def get(self): """Use with the "with" statement; returns a database connection.""" @@ -711,8 +719,12 @@ class DatabaseBroker(object): def get_raw_metadata(self): with self.get() as conn: try: - metadata = conn.execute('SELECT metadata FROM %s_stat' % - self.db_type).fetchone()[0] + row = conn.execute('SELECT metadata FROM %s_stat' % + self.db_type).fetchone() + if not row: + self.quarantine("missing row in %s_stat table" % + self.db_type) + metadata = row[0] except sqlite3.OperationalError as err: if 'no such column: metadata' not in str(err): raise @@ -784,8 +796,12 @@ class DatabaseBroker(object): return with self.get() as conn: try: - md = conn.execute('SELECT metadata FROM %s_stat' % - self.db_type).fetchone()[0] + row = conn.execute('SELECT metadata FROM %s_stat' % + self.db_type).fetchone() + if not row: + self.quarantine("missing row in %s_stat table" % + self.db_type) + md = row[0] md = json.loads(md) if md else {} utf8encodekeys(md) except sqlite3.OperationalError as err: @@ -854,8 +870,12 @@ class DatabaseBroker(object): :returns: True if conn.commit() should be called """ try: - md = conn.execute('SELECT metadata FROM %s_stat' % - self.db_type).fetchone()[0] + row = conn.execute('SELECT metadata FROM %s_stat' % + self.db_type).fetchone() + if not row: + self.quarantine("missing row in %s_stat table" % + self.db_type) + md = row[0] if md: md = json.loads(md) keys_to_delete = [] diff --git a/test/unit/common/missing_container_info.db b/test/unit/common/missing_container_info.db new file mode 100644 index 0000000000..cd9ee50ffc Binary files /dev/null and b/test/unit/common/missing_container_info.db differ diff --git a/test/unit/common/test_db.py b/test/unit/common/test_db.py index f147b30562..f605d0acba 100644 --- a/test/unit/common/test_db.py +++ b/test/unit/common/test_db.py @@ -791,6 +791,29 @@ class TestDatabaseBroker(unittest.TestCase): 'Quarantined %s to %s due to corrupted database' % (dbpath, qpath)) + def test_get_raw_metadata_missing_container_info(self): + # Test missing container_info/container_stat row + dbpath = os.path.join(self.testdir, 'dev', 'dbs', 'par', 'pre', 'db') + mkdirs(dbpath) + qpath = os.path.join(self.testdir, 'dev', 'quarantined', 'containers', + 'db') + copy(os.path.join(os.path.dirname(__file__), + 'missing_container_info.db'), + os.path.join(dbpath, '1.db')) + + broker = DatabaseBroker(os.path.join(dbpath, '1.db')) + broker.db_type = 'container' + + exc = None + try: + broker.get_raw_metadata() + except Exception as err: + exc = err + self.assertEqual( + str(exc), + 'Quarantined %s to %s due to missing row in container_stat table' % + (dbpath, qpath)) + def test_lock(self): broker = DatabaseBroker(os.path.join(self.testdir, '1.db'), timeout=.1) got_exc = False @@ -1133,6 +1156,55 @@ class TestDatabaseBroker(unittest.TestCase): [first_value, first_timestamp]) self.assertNotIn('Second', broker.metadata) + def test_update_metadata_missing_container_info(self): + # Test missing container_info/container_stat row + dbpath = os.path.join(self.testdir, 'dev', 'dbs', 'par', 'pre', 'db') + mkdirs(dbpath) + qpath = os.path.join(self.testdir, 'dev', 'quarantined', 'containers', + 'db') + copy(os.path.join(os.path.dirname(__file__), + 'missing_container_info.db'), + os.path.join(dbpath, '1.db')) + + broker = DatabaseBroker(os.path.join(dbpath, '1.db')) + broker.db_type = 'container' + + exc = None + try: + first_timestamp = normalize_timestamp(1) + first_value = '1' + broker.update_metadata({'First': [first_value, first_timestamp]}) + except Exception as err: + exc = err + self.assertEqual( + str(exc), + 'Quarantined %s to %s due to missing row in container_stat table' % + (dbpath, qpath)) + + def test_reclaim_missing_container_info(self): + # Test missing container_info/container_stat row + dbpath = os.path.join(self.testdir, 'dev', 'dbs', 'par', 'pre', 'db') + mkdirs(dbpath) + qpath = os.path.join(self.testdir, 'dev', 'quarantined', 'containers', + 'db') + copy(os.path.join(os.path.dirname(__file__), + 'missing_container_info.db'), + os.path.join(dbpath, '1.db')) + + broker = DatabaseBroker(os.path.join(dbpath, '1.db')) + broker.db_type = 'container' + + exc = None + try: + with broker.get() as conn: + broker._reclaim(conn, 0) + except Exception as err: + exc = err + self.assertEqual( + str(exc), + 'Quarantined %s to %s due to missing row in container_stat table' % + (dbpath, qpath)) + @patch.object(DatabaseBroker, 'validate_metadata') def test_validate_metadata_is_called_from_update_metadata(self, mock): broker = self.get_replication_info_tester(metadata=True)