Skip to content

Commit

Permalink
issue-2076: fix affected disk tracking
Browse files Browse the repository at this point in the history
  • Loading branch information
@sharpeye
sharpeye committed Sep 24, 2024
1 parent 27d09e5 commit b698180
Show file tree
Hide file tree
Showing 2 changed files with 257 additions and 16 deletions.
24 changes: 9 additions & 15 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 @@ -1208,7 +1208,7 @@ NProto::TError TDiskRegistryState::ReplaceDevice(
timestamp,
message);
if (HasError(error)) {
TryUpdateDiskState(db, diskId, timestamp);
*diskStateUpdated = TryUpdateDiskState(db, diskId, timestamp);
return error;
}

Expand Down Expand Up @@ -4749,7 +4749,7 @@ void TDiskRegistryState::ApplyAgentStateChange(
UpdateAgent(db, agent);
DeviceList.UpdateDevices(agent, DevicePoolConfigs);

THashSet<TString> diskIds;
THashMap<TString, NProto::EDiskState> maybeAffectedDisks;

for (const auto& d: agent.GetDevices()) {
const auto& deviceId = d.GetDeviceUUID();
Expand All @@ -4761,13 +4761,13 @@ void TDiskRegistryState::ApplyAgentStateChange(

auto& disk = Disks[diskId];

maybeAffectedDisks.emplace(diskId, disk.State);

// check if deviceId is target for migration
if (RestartDeviceMigration(timestamp, db, diskId, disk, deviceId)) {
continue;
}

bool isAffected = true;

if (agent.GetState() == NProto::AGENT_STATE_WARNING) {
if (disk.MigrationSource2Target.contains(deviceId)) {
// migration already started
Expand Down Expand Up @@ -4818,24 +4818,18 @@ void TDiskRegistryState::ApplyAgentStateChange(
ReportMirroredDiskDeviceReplacementFailure(
FormatError(error));
}

if (!updated) {
isAffected = false;
}
}
}

CancelDeviceMigration(timestamp, db, diskId, disk, deviceId);
}

if (isAffected) {
diskIds.emplace(std::move(diskId));
}
}

for (auto& id: diskIds) {
if (TryUpdateDiskState(db, id, timestamp)) {
affectedDisks.push_back(std::move(id));
for (auto& [diskId, oldState]: maybeAffectedDisks) {
auto& disk = Disks[diskId];
TryUpdateDiskState(db, diskId, disk, timestamp);
if (oldState != disk.State) {
affectedDisks.push_back(diskId);
}
}
}
Expand Down
249 changes: 248 additions & 1 deletion cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_mirrored_disks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1806,6 +1806,129 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMirroredDisksTest)
rt);
}

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

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

auto agentConfig2 = AgentConfig(2, {
Device("dev-1", "uuid-3", "rack-2"),
Device("dev-2", "uuid-4", "rack-2"),
});

auto agentConfig3 = AgentConfig(3, {
Device("dev-1", "uuid-5", "rack-3"),
});

TDiskRegistryState state = TDiskRegistryStateBuilder()
.WithKnownAgents({
agentConfig1,
agentConfig2,
agentConfig3
})
.Build();

// Create a mirror-2 disk
executor.WriteTx([&] (TDiskRegistryDatabase db) {
TVector<TDeviceConfig> devices;
TVector<TVector<TDeviceConfig>> replicas;
TVector<NProto::TDeviceMigration> migrations;
TVector<TString> deviceReplacementIds;
auto error = AllocateMirroredDisk(
db,
state,
"disk-1",
20_GB,
1,
devices,
replicas,
migrations,
deviceReplacementIds);
UNIT_ASSERT_SUCCESS(error);
UNIT_ASSERT_VALUES_EQUAL(2, devices.size());
UNIT_ASSERT_VALUES_EQUAL("uuid-1", devices[0].GetDeviceUUID());
UNIT_ASSERT_VALUES_EQUAL("uuid-2", devices[1].GetDeviceUUID());
UNIT_ASSERT_VALUES_EQUAL(1, replicas.size());
UNIT_ASSERT_VALUES_EQUAL(2, replicas[0].size());
UNIT_ASSERT_VALUES_EQUAL("uuid-3", replicas[0][0].GetDeviceUUID());
UNIT_ASSERT_VALUES_EQUAL("uuid-4", replicas[0][1].GetDeviceUUID());
ASSERT_VECTORS_EQUAL(TVector<TString>{}, deviceReplacementIds);
});

// All replicas must be online

{
TDiskInfo replicaInfo;
UNIT_ASSERT_SUCCESS(state.GetDiskInfo("disk-1/0", replicaInfo));
UNIT_ASSERT_VALUES_EQUAL(
NProto::EDiskState_Name(NProto::DISK_STATE_ONLINE),
NProto::EDiskState_Name(replicaInfo.State));
}

{
TDiskInfo replicaInfo;
UNIT_ASSERT_SUCCESS(state.GetDiskInfo("disk-1/1", replicaInfo));
UNIT_ASSERT_VALUES_EQUAL(
NProto::EDiskState_Name(NProto::DISK_STATE_ONLINE),
NProto::EDiskState_Name(replicaInfo.State));
}

// Breaking the first agent

executor.WriteTx([&] (TDiskRegistryDatabase db) mutable {
TVector<TString> affectedDisks;
TDuration timeout;
auto error = state.UpdateAgentState(
db,
agentConfig1.GetAgentId(),
NProto::AGENT_STATE_UNAVAILABLE,
Now(),
"unreachable",
affectedDisks);

// Now first replica (disk-1/0) should be broken

TDiskInfo replicaInfo;
UNIT_ASSERT_SUCCESS(state.GetDiskInfo("disk-1/0", replicaInfo));
UNIT_ASSERT_VALUES_EQUAL(
NProto::EDiskState_Name(NProto::DISK_STATE_TEMPORARILY_UNAVAILABLE),
NProto::EDiskState_Name(replicaInfo.State));

// And second one (disk-1/1) should be OK

replicaInfo = {};
UNIT_ASSERT_SUCCESS(state.GetDiskInfo("disk-1/1", replicaInfo));
UNIT_ASSERT_VALUES_EQUAL(
NProto::EDiskState_Name(NProto::DISK_STATE_ONLINE),
NProto::EDiskState_Name(replicaInfo.State));

UNIT_ASSERT_VALUES_EQUAL(S_OK, error.GetCode());
UNIT_ASSERT_VALUES_EQUAL(TDuration::Zero(), timeout);

// The state of 'disk-1/0' has been changed so 'disk-1/0' should be
// in 'affectedDisks'
ASSERT_VECTORS_EQUAL(TVector<TString>{"disk-1/0"}, affectedDisks);

auto replaced = state.GetAutomaticallyReplacedDevices();
UNIT_ASSERT_VALUES_EQUAL(1, replaced.size());
UNIT_ASSERT_VALUES_EQUAL("uuid-1", replaced[0].DeviceId);

UNIT_ASSERT_SUCCESS(state.MarkReplacementDevice(
Now(),
db,
"disk-1",
"uuid-5",
false));
});
}

Y_UNIT_TEST(ShouldProperlyCleanupAutomaticallyReplacedDevices)
{
TTestExecutor executor;
Expand Down Expand Up @@ -2400,6 +2523,13 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMirroredDisksTest)
const auto changeStateTs = Now();

executor.WriteTx([&] (TDiskRegistryDatabase db) mutable {
TDiskInfo diskInfo;
UNIT_ASSERT_SUCCESS(state.GetDiskInfo("disk-1/0", diskInfo));
UNIT_ASSERT_VALUES_EQUAL(
NProto::EDiskState_Name(
NProto::DISK_STATE_ONLINE),
NProto::EDiskState_Name(diskInfo.State));

TVector<TString> affectedDisks;
TDuration timeout;
auto error = state.UpdateAgentState(
Expand All @@ -2412,7 +2542,14 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMirroredDisksTest)

UNIT_ASSERT_VALUES_EQUAL(S_OK, error.GetCode());
UNIT_ASSERT_VALUES_EQUAL(TDuration::Zero(), timeout);
UNIT_ASSERT_VALUES_EQUAL(0, affectedDisks.size());
ASSERT_VECTORS_EQUAL(TVector<TString>{"disk-1/0"}, affectedDisks);

diskInfo = {};
UNIT_ASSERT_SUCCESS(state.GetDiskInfo("disk-1/0", diskInfo));
UNIT_ASSERT_VALUES_EQUAL(
NProto::EDiskState_Name(
NProto::DISK_STATE_TEMPORARILY_UNAVAILABLE),
NProto::EDiskState_Name(diskInfo.State));
});

UNIT_ASSERT_VALUES_EQUAL(1, criticalEvents->Val());
Expand Down Expand Up @@ -2482,6 +2619,116 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMirroredDisksTest)
});
}

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

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

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

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

TDiskRegistryState state = TDiskRegistryStateBuilder()
.WithKnownAgents({
agentConfig1,
agentConfig2,
agentConfig3,
})
.Build();

// Creating a mirror-3 (one device per replica)

executor.WriteTx([&] (TDiskRegistryDatabase db) {
TVector<TDeviceConfig> devices;
TVector<TVector<TDeviceConfig>> replicas;
TVector<NProto::TDeviceMigration> migrations;
TVector<TString> deviceReplacementIds;
auto error = AllocateMirroredDisk(
db,
state,
"disk-1",
10_GB,
2,
devices,
replicas,
migrations,
deviceReplacementIds);
UNIT_ASSERT_SUCCESS(error);
UNIT_ASSERT_VALUES_EQUAL(1, devices.size());
UNIT_ASSERT_VALUES_EQUAL("uuid-1", devices[0].GetDeviceUUID());
UNIT_ASSERT_VALUES_EQUAL(2, replicas.size());
UNIT_ASSERT_VALUES_EQUAL(1, replicas[0].size());
UNIT_ASSERT_VALUES_EQUAL("uuid-2", replicas[0][0].GetDeviceUUID());
UNIT_ASSERT_VALUES_EQUAL(1, replicas[1].size());
UNIT_ASSERT_VALUES_EQUAL("uuid-3", replicas[1][0].GetDeviceUUID());
ASSERT_VECTORS_EQUAL(TVector<TString>{}, deviceReplacementIds);
});

auto monitoring = CreateMonitoringServiceStub();
auto rootGroup = monitoring->GetCounters()
->GetSubgroup("counters", "blockstore");

auto serverGroup = rootGroup->GetSubgroup("component", "server");
InitCriticalEventsCounter(serverGroup);

auto criticalEvents = serverGroup->FindCounter(
"AppCriticalEvents/MirroredDiskDeviceReplacementFailure");

UNIT_ASSERT_VALUES_EQUAL(0, criticalEvents->Val());

executor.WriteTx([&] (TDiskRegistryDatabase db) mutable {

// A first replica should be OK

TDiskInfo diskInfo;
UNIT_ASSERT_SUCCESS(state.GetDiskInfo("disk-1/0", diskInfo));
UNIT_ASSERT_VALUES_EQUAL(
NProto::EDiskState_Name(
NProto::DISK_STATE_ONLINE),
NProto::EDiskState_Name(diskInfo.State));

// Breakin a device

TString affectedDisk;
TDuration timeout;
auto error = state.UpdateDeviceState(
db,
"uuid-1",
NProto::DEVICE_STATE_ERROR,
Now(),
"test",
affectedDisk);

// Now the first replca is not OK, its state has been changed to
// ERROR so we should see disk-1/0 in affectedDisk

UNIT_ASSERT_VALUES_EQUAL(S_OK, error.GetCode());
UNIT_ASSERT_VALUES_EQUAL(TDuration::Zero(), timeout);
UNIT_ASSERT_VALUES_EQUAL("disk-1/0", affectedDisk);

// Ensuring that disk-1/0 is broken

diskInfo = {};
UNIT_ASSERT_SUCCESS(state.GetDiskInfo("disk-1/0", diskInfo));
UNIT_ASSERT_VALUES_EQUAL(
NProto::EDiskState_Name(
NProto::DISK_STATE_ERROR),
NProto::EDiskState_Name(diskInfo.State));
});

UNIT_ASSERT_VALUES_EQUAL(1, criticalEvents->Val());
}

Y_UNIT_TEST(ShouldStopAutomaticReplacementIfReplacementRateIsTooHigh)
{
TTestExecutor executor;
Expand Down

0 comments on commit b698180

Please sign in to comment.