Skip to content

Commit

Permalink
issue 1394: optimize cleanup devices (#1476)
Browse files Browse the repository at this point in the history 
Issue: #1394

1. Pass all devices as an argument for MarkDevicesAsClean
2. Update each agent only once.

TryUpdateDevices iterates via devices and save agents in the map to call UpdateAgent and DeviceList.UpdateDevices for each agent only once.
  • Loading branch information
@antonmyagkov
antonmyagkov committed Jul 22, 2024
1 parent a478d50 commit 6574310
Show file tree
Hide file tree
Showing 9 changed files with 208 additions and 68 deletions.
11 changes: 2 additions & 9 deletions cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_secure_erase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,16 +278,9 @@ void TDiskRegistryActor::ExecuteCleanupDevices(
TTransactionContext& tx,
TTxDiskRegistry::TCleanupDevices& args)
{
Y_UNUSED(ctx);

TDiskRegistryDatabase db(tx.DB);

for (const auto& uuid: args.Devices) {
auto diskId = State->MarkDeviceAsClean(ctx.Now(), db, uuid);
if (diskId) {
args.SyncDeallocatedDisks.push_back(std::move(diskId));
}
}
args.SyncDeallocatedDisks =
State->MarkDevicesAsClean(ctx.Now(), db, args.Devices);
}

void TDiskRegistryActor::CompleteCleanupDevices(
Expand Down
67 changes: 51 additions & 16 deletions cloud/blockstore/libs/storage/disk_registry/disk_registry_state.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3601,41 +3601,76 @@ bool TDiskRegistryState::MarkDeviceAsDirty(
return true;
}

TString TDiskRegistryState::MarkDeviceAsClean(
TDiskRegistryState::TDiskId TDiskRegistryState::MarkDeviceAsClean(
TInstant now,
TDiskRegistryDatabase& db,
const TDeviceId& uuid)
{
DeviceList.MarkDeviceAsClean(uuid);
db.DeleteDirtyDevice(uuid);
auto ret = MarkDevicesAsClean(now, db, TVector<TDeviceId>{uuid});
return ret.empty() ? "" : ret[0];
}

if (!DeviceList.IsSuspendedDevice(uuid)) {
db.DeleteSuspendedDevice(uuid);
TVector<TDiskRegistryState::TDiskId> TDiskRegistryState::MarkDevicesAsClean(
TInstant now,
TDiskRegistryDatabase& db,
const TVector<TDeviceId>& uuids)
{
for (const auto& uuid: uuids) {
DeviceList.MarkDeviceAsClean(uuid);
db.DeleteDirtyDevice(uuid);

if (!DeviceList.IsSuspendedDevice(uuid)) {
db.DeleteSuspendedDevice(uuid);
}
}

TryUpdateDevice(now, db, uuid);
TVector<TDiskId> ret;
for (const auto& uuid: TryUpdateDevices(now, db, uuids)) {
if (auto diskId = PendingCleanup.EraseDevice(uuid); !diskId.empty()) {
ret.push_back(std::move(diskId));
}
}

return PendingCleanup.EraseDevice(uuid);
return ret;
}

bool TDiskRegistryState::TryUpdateDevice(
TInstant now,
TDiskRegistryDatabase& db,
const TDeviceId& uuid)
{
Y_UNUSED(now);
return !TryUpdateDevices(now, db, {uuid}).empty();
}

auto [agent, device] = FindDeviceLocation(uuid);
if (!agent || !device) {
return false;
}
TVector<TDiskRegistryState::TDeviceId> TDiskRegistryState::TryUpdateDevices(
TInstant now,
TDiskRegistryDatabase& db,
const TVector<TDeviceId>& uuids)
{
TVector<TDeviceId> ret;
ret.reserve(uuids.size());

AdjustDeviceIfNeeded(*device, {});
TSet<TAgentId> agentsMap;
for (const auto& uuid: uuids) {
auto [agent, device] = FindDeviceLocation(uuid);
if (!agent || !device) {
continue;
}
ret.push_back(uuid);
agentsMap.emplace(agent->agentid());
AdjustDeviceIfNeeded(*device, now);
}

UpdateAgent(db, *agent);
DeviceList.UpdateDevices(*agent, DevicePoolConfigs);
for (const auto& agentId: agentsMap) {
auto* agent = AgentList.FindAgent(agentId);
if (!agent) {
continue;
}
UpdateAgent(db, *agent);
DeviceList.UpdateDevices(*agent, DevicePoolConfigs);
}

return true;
return ret;
}

TVector<TString> TDiskRegistryState::CollectBrokenDevices(
Expand Down
25 changes: 24 additions & 1 deletion cloud/blockstore/libs/storage/disk_registry/disk_registry_state.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,10 +493,22 @@ class TDiskRegistryState
TVector<NProto::TDeviceConfig> GetBrokenDevices() const;

TVector<NProto::TDeviceConfig> GetDirtyDevices() const;
TString MarkDeviceAsClean(

/// Mark selected device as clean and remove it
/// from lists of suspended/dirty/pending cleanup devices
/// @return disk id where selected device was allocated
TDiskId MarkDeviceAsClean(
TInstant now,
TDiskRegistryDatabase& db,
const TDeviceId& uuid);

/// Mark selected devices as clean and remove them
/// from lists of suspended/dirty/pending cleanup devices
/// @return vector of disk ids where selected devices were allocated
TVector<TDiskId> MarkDevicesAsClean(
TInstant now,
TDiskRegistryDatabase& db,
const TVector<TDeviceId>& uuids);
bool MarkDeviceAsDirty(TDiskRegistryDatabase& db, const TDeviceId& uuid);

NProto::TError CreatePlacementGroup(
Expand Down Expand Up @@ -1123,11 +1135,22 @@ class TDiskRegistryState
TDiskRegistryDatabase& db,
const TString& diskId);

/// Try to update configuration of selected device and its agent
/// in the disk registry database
/// @return true if the device updates successfully; otherwise, return false
bool TryUpdateDevice(
TInstant now,
TDiskRegistryDatabase& db,
const TDeviceId& uuid);

/// Try to update configuration of selected devices and their agents
/// in the disk registry database
/// @return List of updated devices
TVector<TDeviceId> TryUpdateDevices(
TInstant now,
TDiskRegistryDatabase& db,
const TVector<TDeviceId>& uuids);

TDeviceList::TAllocationQuery MakeMigrationQuery(
const TDiskId& sourceDiskId,
const NProto::TDeviceConfig& sourceDevice);
Expand Down
125 changes: 108 additions & 17 deletions cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1271,11 +1271,13 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateTest)
Device("dev-3", "uuid-7", "rack-1")
});

TDiskRegistryState state = TDiskRegistryStateBuilder()
.WithKnownAgents({ agent1, agent2 })
.WithDisks({ Disk("disk-1", {"uuid-1"}) })
.WithDirtyDevices({ "uuid-4", "uuid-7" })
.Build();
TDiskRegistryState state =
TDiskRegistryStateBuilder()
.WithKnownAgents({agent1, agent2})
.WithDisks({Disk("disk-1", {"uuid-1"})})
.WithDirtyDevices(
{TDirtyDevice{"uuid-4", {}}, TDirtyDevice{"uuid-7", {}}})
.Build();

executor.WriteTx([&] (TDiskRegistryDatabase db) {
TVector<TDeviceConfig> devices;
Expand Down Expand Up @@ -8292,18 +8294,29 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateTest)
}),
};

TDiskRegistryState state = TDiskRegistryStateBuilder()
.WithKnownAgents(agents)
.WithDisks({
Disk("disk-1", {"uuid-1.1"}),
Disk("disk-2", {"uuid-2.2"}),
Disk("disk-3", {"uuid-3.4", "uuid-3.5"}),
Disk("disk-4", {"uuid-4.1"}),
})
.WithDirtyDevices({"uuid-2.1", "uuid-3.3", "uuid-3.6", "uuid-5.2"})
.AddDevicePoolConfig("local-ssd", 10_GB, NProto::DEVICE_POOL_KIND_LOCAL)
.AddDevicePoolConfig("rot", 10_GB, NProto::DEVICE_POOL_KIND_GLOBAL)
.Build();
TDiskRegistryState state =
TDiskRegistryStateBuilder()
.WithKnownAgents(agents)
.WithDisks({
Disk("disk-1", {"uuid-1.1"}),
Disk("disk-2", {"uuid-2.2"}),
Disk("disk-3", {"uuid-3.4", "uuid-3.5"}),
Disk("disk-4", {"uuid-4.1"}),
})
.WithDirtyDevices(
{TDirtyDevice{"uuid-2.1", {}},
TDirtyDevice{"uuid-3.3", {}},
TDirtyDevice{"uuid-3.6", {}},
TDirtyDevice{"uuid-5.2", {}}})
.AddDevicePoolConfig(
"local-ssd",
10_GB,
NProto::DEVICE_POOL_KIND_LOCAL)
.AddDevicePoolConfig(
"rot",
10_GB,
NProto::DEVICE_POOL_KIND_GLOBAL)
.Build();

const auto poolNames = state.GetPoolNames();
ASSERT_VECTORS_EQUAL(
Expand Down Expand Up @@ -11535,6 +11548,84 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateTest)
UNIT_ASSERT_VALUES_EQUAL(errorMessage, d.GetStateMessage());
});
}

Y_UNIT_TEST(ShouldCleanMultipleDevicesFromOneDisk)
{
TTestExecutor executor;
executor.WriteTx([&](TDiskRegistryDatabase db) { db.InitSchema(); });

const auto agent1 = AgentConfig(
1,
{Device("dev-1", "uuid-1", "rack-1")});

const auto agent2 = AgentConfig(
2,
{Device("dev-2", "uuid-2", "rack-1")});

TDiskRegistryState state =
TDiskRegistryStateBuilder()
.WithKnownAgents({agent1, agent2})
.WithDisks({Disk("disk-1", {"uuid-1", "uuid-2"})})
.WithDirtyDevices(
{TDirtyDevice{"uuid-1", "disk-1"},
TDirtyDevice{"uuid-2", "disk-1"}})
.Build();

UNIT_ASSERT_EQUAL(state.GetDirtyDevices().size(), 2);

executor.WriteTx(
[&](TDiskRegistryDatabase db)
{
const auto& cleanDisks = state.MarkDevicesAsClean(
Now(),
db,
TVector<TString>{"uuid-1", "uuid-2"});
UNIT_ASSERT_EQUAL(cleanDisks.size(), 1);
UNIT_ASSERT_EQUAL(cleanDisks[0], "disk-1");
});

UNIT_ASSERT(state.GetDirtyDevices().empty());
}

Y_UNIT_TEST(ShouldCleanMultipleDevicesFromDifferentDisks)
{
TTestExecutor executor;
executor.WriteTx([&](TDiskRegistryDatabase db) { db.InitSchema(); });

const auto agent1 = AgentConfig(
1,
{Device("dev-1", "uuid-1", "rack-1")});

const auto agent2 = AgentConfig(
2,
{Device("dev-2", "uuid-2", "rack-1")});

TDiskRegistryState state =
TDiskRegistryStateBuilder()
.WithKnownAgents({agent1, agent2})
.WithDisks(
{Disk("disk-1", {"uuid-1"}), Disk("disk-1", {"uuid-2"})})
.WithDirtyDevices(
{TDirtyDevice{"uuid-1", "disk-1"},
TDirtyDevice{"uuid-2", "disk-2"}})
.Build();

UNIT_ASSERT_EQUAL(state.GetDirtyDevices().size(), 2);

executor.WriteTx(
[&](TDiskRegistryDatabase db)
{
const auto& cleanDisks = state.MarkDevicesAsClean(
Now(),
db,
TVector<TString>{"uuid-1", "uuid-2"});
UNIT_ASSERT_EQUAL(cleanDisks.size(), 2);
UNIT_ASSERT_EQUAL(cleanDisks[0], "disk-1");
UNIT_ASSERT_EQUAL(cleanDisks[1], "disk-2");
});

UNIT_ASSERT(state.GetDirtyDevices().empty());
}
}

} // namespace NCloud::NBlockStore::NStorage
2 changes: 1 addition & 1 deletion cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_agents_info.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateQueryAgentsInfoTest)
return config;
}())
.WithAgents(agents)
.WithDirtyDevices({"uuid-1.1"})
.WithDirtyDevices({TDirtyDevice{"uuid-1.1", {}}})
.Build();

auto agentsInfo = state.QueryAgentsInfo();
Expand Down
11 changes: 6 additions & 5 deletions cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_create.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,12 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateCreateTest)
})
};

TDiskRegistryState state = TDiskRegistryStateBuilder()
.WithKnownAgents(agents)
.WithDisks({ Disk("disk-1", {"uuid-1.1"}) })
.WithDirtyDevices({"uuid-2.1"})
.Build();
TDiskRegistryState state =
TDiskRegistryStateBuilder()
.WithKnownAgents(agents)
.WithDisks({Disk("disk-1", {"uuid-1.1"})})
.WithDirtyDevices({TDirtyDevice{"uuid-2.1", {}}})
.Build();

auto deviceByName = [] (auto agentId, auto name) {
NProto::TDeviceConfig config;
Expand Down
20 changes: 12 additions & 8 deletions cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_migration.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,18 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
})
};

TDiskRegistryState state = TDiskRegistryStateBuilder()
.WithKnownAgents(agents)
.WithDisks({
Disk("foo", { "uuid-1.1", "uuid-1.2" }), // rack-1
Disk("bar", { "uuid-2.1" }) // rack-2
})
.WithDirtyDevices({"uuid-4.1", "uuid-4.2", "uuid-4.3"})
.Build();
TDiskRegistryState state =
TDiskRegistryStateBuilder()
.WithKnownAgents(agents)
.WithDisks({
Disk("foo", {"uuid-1.1", "uuid-1.2"}), // rack-1
Disk("bar", {"uuid-2.1"}) // rack-2
})
.WithDirtyDevices(
{TDirtyDevice{"uuid-4.1", {}},
TDirtyDevice{"uuid-4.2", {}},
TDirtyDevice{"uuid-4.3", {}}})
.Build();

UNIT_ASSERT(state.IsMigrationListEmpty());

Expand Down
13 changes: 3 additions & 10 deletions cloud/blockstore/libs/storage/disk_registry/testlib/test_state.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,16 +561,9 @@ TDiskRegistryStateBuilder& TDiskRegistryStateBuilder::WithDisks(
}

TDiskRegistryStateBuilder& TDiskRegistryStateBuilder::WithDirtyDevices(
TVector<TString> dirtyDevices)
{
DirtyDevices.clear();
DirtyDevices.reserve(dirtyDevices.size());
for (auto& s: dirtyDevices) {
DirtyDevices.emplace_back(TDirtyDevice {
.Id = std::move(s)
});
}

TVector<TDirtyDevice> dirtyDevices)
{
DirtyDevices = std::move(dirtyDevices);
return *this;
}

Expand Down
2 changes: 1 addition & 1 deletion cloud/blockstore/libs/storage/disk_registry/testlib/test_state.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ struct TDiskRegistryStateBuilder

TDiskRegistryStateBuilder& WithDisks(TVector<NProto::TDiskConfig> disks);

TDiskRegistryStateBuilder& WithDirtyDevices(TVector<TString> dirtyDevices);
TDiskRegistryStateBuilder& WithDirtyDevices(TVector<TDirtyDevice> dirtyDevices);

TDiskRegistryStateBuilder& WithSuspendedDevices(
TVector<TString> suspendedDevices);
Expand Down

0 comments on commit 6574310

Please sign in to comment.