From 08da83c193126f480e6690a83c41695f6f40fcbf Mon Sep 17 00:00:00 2001 From: Matthew Oliver Date: Mon, 7 Feb 2022 14:51:42 +1100 Subject: [PATCH] DB: Encode the device to the DB id Currently the DB ids, used to identify a database broker, is a uuid (uuid4). This is a great unique identifier, however we don't tend to track DB ids. When we replicate to other primaries or nodes. We track which brokers we replicate with or replicate with us via the incoming_sync and outgoing_sync tables. These contain the DB id of the broker in question and a timestamp. These tables entries do get reclaimed over time, so gives us a nice snapshot of whose been replicating with us. Unfortuantly however, as we don't track these UUIDs to a broker or node when we want to go look at these tables in search for replication problems we tend to be at a loss. This patch helps to identify the source by adding the device to the DB id: - The patch uses the device as written in the path of the broker. Now when we look into the sync tables we can look up in the ring and know exactly which node it came from (or talking to). Although this only helps if you use unique device names across the cluster. An ID is generated when an account or container is first initialised. But also after an rsync or rsyn_then_merge on the RPC (new location) side. So as things replicate we'll get updated device's in the ID, which will also show up in syncs in the future as the new node starts to replicate. When applied to an existing cluster, there will be a mix of and IDs. But these ids are just text, and they will slowly change as things get replicated/rebalanced around. Change-Id: I58063c3830fb7b01c26fe1fa5aaec6353ff555c6 --- swift/account/backend.py | 3 +-- swift/common/db.py | 6 ++++- swift/container/backend.py | 2 +- test/unit/account/test_backend.py | 35 +++++++++++++++++++++++++++++ test/unit/container/test_backend.py | 31 +++++++++++++++++++++++-- 5 files changed, 71 insertions(+), 6 deletions(-) diff --git a/swift/account/backend.py b/swift/account/backend.py index cb1eeb5863..ec1de03d15 100644 --- a/swift/account/backend.py +++ b/swift/account/backend.py @@ -16,7 +16,6 @@ Pluggable Back-end for Account Server """ -from uuid import uuid4 import sqlite3 @@ -154,7 +153,7 @@ class AccountBroker(DatabaseBroker): conn.execute(''' UPDATE account_stat SET account = ?, created_at = ?, id = ?, put_timestamp = ?, status_changed_at = ? - ''', (self.account, Timestamp.now().internal, str(uuid4()), + ''', (self.account, Timestamp.now().internal, self._new_db_id(), put_timestamp, put_timestamp)) def create_policy_stat_table(self, conn): diff --git a/swift/common/db.py b/swift/common/db.py index 7962e461f4..3e42d952ae 100644 --- a/swift/common/db.py +++ b/swift/common/db.py @@ -592,6 +592,10 @@ class DatabaseBroker(object): _('Broker error trying to rollback locked connection')) conn.close() + def _new_db_id(self): + device_name = os.path.basename(self.get_device_path()) + return "%s-%s" % (str(uuid4()), device_name) + def newid(self, remote_id): """ Re-id the database. This should be called after an rsync. @@ -601,7 +605,7 @@ class DatabaseBroker(object): with self.get() as conn: row = conn.execute(''' UPDATE %s_stat SET id=? - ''' % self.db_type, (str(uuid4()),)) + ''' % self.db_type, (self._new_db_id(),)) row = conn.execute(''' SELECT ROWID FROM %s ORDER BY ROWID DESC LIMIT 1 ''' % self.db_contains_type).fetchone() diff --git a/swift/container/backend.py b/swift/container/backend.py index 0e062e9429..6c0e552787 100644 --- a/swift/container/backend.py +++ b/swift/container/backend.py @@ -579,7 +579,7 @@ class ContainerBroker(DatabaseBroker): put_timestamp, status_changed_at, storage_policy_index) VALUES (?, ?, ?, ?, ?, ?, ?); """, (self.account, self.container, Timestamp.now().internal, - str(uuid4()), put_timestamp, put_timestamp, + self._new_db_id(), put_timestamp, put_timestamp, storage_policy_index)) def create_policy_stat_table(self, conn, storage_policy_index=0): diff --git a/test/unit/account/test_backend.py b/test/unit/account/test_backend.py index 8d6d6a954d..1ba963890a 100644 --- a/test/unit/account/test_backend.py +++ b/test/unit/account/test_backend.py @@ -30,6 +30,7 @@ from contextlib import contextmanager import random import mock import base64 +import shutil import six @@ -1071,6 +1072,40 @@ class TestAccountBroker(unittest.TestCase): "SELECT COUNT(*) FROM policy_stat").fetchall()[0][0] self.assertEqual(nrows, 2) + @with_tempdir + def test_newid(self, tempdir): + # test DatabaseBroker.newid + db_path = os.path.join( + tempdir, "d1234", 'accounts', 'part', 'suffix', 'hsh') + os.makedirs(db_path) + broker = AccountBroker(os.path.join(db_path, 'my.db'), + account='a') + broker.initialize(Timestamp('1').internal, 0) + id = broker.get_info()['id'] + broker.newid('someid') + self.assertNotEqual(id, broker.get_info()['id']) + # ends in the device name (from the path) unless it's an old + # container with just a uuid4 (tested in legecy broker + # tests e.g *BeforeMetaData) + if len(id) > 36: + self.assertTrue(id.endswith('d1234')) + # But the newid'ed version will now have the decide + self.assertTrue(broker.get_info()['id'].endswith('d1234')) + + # if we move the broker (happens after an rsync) + new_db_path = os.path.join( + tempdir, "d5678", 'contianers', 'part', 'suffix', 'hsh') + os.makedirs(new_db_path) + shutil.copy(os.path.join(db_path, 'my.db'), + os.path.join(new_db_path, 'my.db')) + + new_broker = AccountBroker(os.path.join(new_db_path, 'my.db'), + account='a') + new_broker.newid(id) + # ends in the device name (from the path) + self.assertFalse(new_broker.get_info()['id'].endswith('d1234')) + self.assertTrue(new_broker.get_info()['id'].endswith('d5678')) + def prespi_AccountBroker_initialize(self, conn, put_timestamp, **kwargs): """ diff --git a/test/unit/container/test_backend.py b/test/unit/container/test_backend.py index 85d07d0a97..dd9b92817d 100644 --- a/test/unit/container/test_backend.py +++ b/test/unit/container/test_backend.py @@ -18,6 +18,7 @@ import base64 import errno import os import inspect +import shutil import unittest from time import sleep, time from uuid import uuid4 @@ -3328,13 +3329,39 @@ class TestContainerBroker(unittest.TestCase): hashc = '%032x' % (int(hasha, 16) ^ int(hashb, 16)) self.assertEqual(broker.get_info()['hash'], hashc) - def test_newid(self): + @with_tempdir + def test_newid(self, tempdir): # test DatabaseBroker.newid - broker = ContainerBroker(':memory:', account='a', container='c') + db_path = os.path.join( + tempdir, "d1234", 'contianers', 'part', 'suffix', 'hsh') + os.makedirs(db_path) + broker = ContainerBroker(os.path.join(db_path, 'my.db'), + account='a', container='c') broker.initialize(Timestamp('1').internal, 0) id = broker.get_info()['id'] broker.newid('someid') self.assertNotEqual(id, broker.get_info()['id']) + # ends in the device name (from the path) unless it's an old + # container with just a uuid4 (tested in legecy broker + # tests e.g *BeforeMetaData) + if len(id) > 36: + self.assertTrue(id.endswith('d1234')) + # But the newid'ed version will now have the decide + self.assertTrue(broker.get_info()['id'].endswith('d1234')) + + # if we move the broker (happens after an rsync) + new_db_path = os.path.join( + tempdir, "d5678", 'containers', 'part', 'suffix', 'hsh') + os.makedirs(new_db_path) + shutil.copy(os.path.join(db_path, 'my.db'), + os.path.join(new_db_path, 'my.db')) + + new_broker = ContainerBroker(os.path.join(new_db_path, 'my.db'), + account='a', container='c') + new_broker.newid(id) + # ends in the device name (from the path) + self.assertFalse(new_broker.get_info()['id'].endswith('d1234')) + self.assertTrue(new_broker.get_info()['id'].endswith('d5678')) def test_get_items_since(self): # test DatabaseBroker.get_items_since