Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lagging agent concept to the volume #2524

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

komarevtsev-d
Copy link
Collaborator

@komarevtsev-d komarevtsev-d commented Nov 19, 2024

Первый большой ПР про issue #3. Здесь я добавляю логику в Volume для работы с отстающими репликами.
Текущий код пока не работает. Поэтому никаких фича тоглов я ещё не добавлял. Они будут в следующих ПРах.

Чтобы машинерия завертелась, volume должен получить сообщение TEvVolumePrivate::TEvDeviceTimeoutedRequest от partition. Volume проверяет может ли он сделать агента, которому принадлежит девайс отстающим (lagging). Может в случае если в тех же строках нет других lagging/fresh девайсов.
Далее он сохраняет lagging девайсы в базу и отправляет сообщение в mirror partition. Обработка этого сообщения будет в следущих ПРах.

Также в этом ПРе есть новое сообщение в DR - TEvDiskRegistry::TEvAddLaggingDevicesRequest. Если partition был перезапущен, то вся информация в оперативной памяти о dirty блоках была потеряна. Поэтому нужно уведомить DR о том, что волум имеет lagging девайсы, дабы случайно не получилось минус 3 реплики.
Его логика тоже будет в следующих ПРах.

Дока по фиче здесь: https://github.com/ydb-platform/nbs/blob/main/doc/blockstore/storage/dynamic_io_mirroring_proposal.md

@komarevtsev-d komarevtsev-d added large-tests Launch large tests for PR blockstore Add this label to run only cloud/blockstore build and tests on PR labels Nov 19, 2024
@komarevtsev-d komarevtsev-d force-pushed the users/komarevtsev-d/issue-3/2 branch 3 times, most recently from 21ff8af to 852563f Compare November 22, 2024 05:01
@komarevtsev-d komarevtsev-d force-pushed the users/komarevtsev-d/issue-3/2 branch from 852563f to 99e7632 Compare January 5, 2025 08:57
@ydb-platform ydb-platform deleted a comment from github-actions bot Jan 5, 2025
@ydb-platform ydb-platform deleted a comment from github-actions bot Jan 5, 2025
Copy link
Contributor

github-actions bot commented Jan 5, 2025

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 99e7632.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3757 3757 0 0 0 0

@komarevtsev-d komarevtsev-d force-pushed the users/komarevtsev-d/issue-3/2 branch 2 times, most recently from cfdcc31 to 23b16ca Compare January 5, 2025 12:59
@komarevtsev-d komarevtsev-d marked this pull request as ready for review January 5, 2025 13:02
@komarevtsev-d komarevtsev-d added the ok-to-test Label to approve test launch for external members label Jan 5, 2025
@github-actions github-actions bot removed the ok-to-test Label to approve test launch for external members label Jan 5, 2025
@@ -61,6 +61,8 @@ class TNonreplicatedPartitionConfig
const NActors::TActorId ParentActorId;
const bool MuteIOErrors;
const THashSet<TString> FreshDeviceIds;
// List of devices that have outdated data. Can only appear on mirror disks.
const THashSet<TString> LaggingDeviceIds;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил в partition config список девайсов которые отстают.
Будет использоваться для двух вещей: mirror partition не читает из таких из-за DevicesReadyForReading() и в nonrepl partition тоже будет предохранитель "на всякий случай" от наших сервисных запросов.

@@ -606,9 +618,10 @@ std::unique_ptr<TTestActorRuntime> PrepareTestActorRuntime(
TDiskRegistryStatePtr diskRegistryState,
NProto::TFeaturesConfig featuresConfig,
NRdma::IClientPtr rdmaClient,
TDiskAgentStatePtr diskAgentState)
TVector<TDiskAgentStatePtr> diskAgentStates)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В тестах волума добавил возможность создавать больше 1 диск агента.
Без этого ничего не протестировать нормально, т.к. логика волума расчитывает на то что разные реплики mirror диска не могут содержать девайсы одного агента.

Copy link
Contributor

github-actions bot commented Jan 5, 2025

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 23b16ca.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3757 3757 0 0 0 0

@komarevtsev-d komarevtsev-d requested a review from drbasic January 7, 2025 16:03
@sharpeye sharpeye self-requested a review January 10, 2025 21:00
THashSet<TString>(), // laggingDeviceIds
TDuration::Zero(), // maxTimedOutDeviceStateDuration
false, // maxTimedOutDeviceStateDurationOverridden
true // useSimpleMigrationBandwidthLimiter
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Он так и сделал

cloud/blockstore/libs/storage/protos/disk.proto Outdated Show resolved Hide resolved
for (const auto& laggingDevice: disk.LaggingDevices) {
*response->Record.AddRemovedLaggingDevices() = laggingDevice;
}
disk.LaggingDevices.clear();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а почему тут происходит очистка?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Порядок действий с настоящим DR такой:

  1. Волюм потерял возможность восстановить lagging девайс
  2. Волюм отправляет все такие девайсы в DR
  3. DR для каждого решает что сделать. Обычно он их делает фрешами и запускает реалокацию
  4. В реалокации к волюму приходит список лагающих девайсов которые DR учёл. Волюм удаляет их у себя
  5. DR удаляет лагающие девайсы у себя на ответ от волюма

В тестах проще сразу чистить.

{
std::optional<ui32> replicaIndex;
const RepeatedPtrField<NProto::TDeviceConfig>* replicaDevices = nullptr;
const auto deviceMatcher = [agentNodeId](const NProto::TDeviceConfig& device)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agentMatcher

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так он же девайсы матчит.

}

// There is no devices from desired agent.
if (!replicaIndex || !replicaDevices) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а если агент был только в миграциях, то мы его не добавим.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В 139 строке цикл по миграциям специально для этого случая.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я добавлю тест про это.

cloud/blockstore/libs/storage/volume/model/helpers.cpp Outdated Show resolved Hide resolved
cloud/blockstore/libs/storage/volume/model/helpers.cpp Outdated Show resolved Hide resolved
string DeviceUUID = 1;

// Index of the lagging device in the replica.
uint32 RowIndex = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

может удобнее тут иметь индекс реплики ?
uint32 ReplicaIndex = 3;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я чет не придумал где оно сильно пригодиться может

@komarevtsev-d komarevtsev-d force-pushed the users/komarevtsev-d/issue-3/2 branch from 720e631 to 68ae225 Compare January 14, 2025 12:27
@komarevtsev-d komarevtsev-d requested a review from drbasic January 14, 2025 12:33
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 68ae225.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3769 3767 0 2 0 0

for (const auto& laggingAgent:
State->GetMeta().GetLaggingAgentsInfo().GetAgents())
{
for (const auto& laggingDevice: laggingAgent.GetDevices()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а каким механизмом мы будем говорить DR что агент больше не лагает?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мы не будем.
Лагающие отправляем в DR, а он тут же их назначает fresh'ами и отдаёт нам новый конфиг. Волум чистит лагающие девайсы у себя и DR у себя

laggingAgent.GetAgentId().c_str());

STORAGE_CHECK_PRECONDITION(
laggingAgent.GetDevices().size() ==
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ресайз лагающего диска не нарушит инвариант?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ресайз это обновление конфига. Мы на него позовём UpdateLaggingDevicesAfterMetaUpdate и там всё "починится"

timeoutedAgentDevices.ysize());
NCloud::Send(
ctx,
State->GetDiskRegistryBasedPartitionActor(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AddLaggingAgent шлем в партишен, тем самым подтверждаем что мы уже считаем агент лагающим. а почему может получится что мы уже внесли агента в это состояние?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Внутри партишиона будут 2 состояние у лагающего:

  1. недоступен - когда агент не отвечает
  2. наливка - когда агент проснулся и мы наливаем отстающие блоки.

Но в процессе наливки он опять может залагать

}

// Check for fresh devices in the same row.
for (const auto& laggingDevice: timeoutedAgentDevices) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

может в утилитах должна быть какая-то функция, отвечающая на вопрос можно ли сделать девайс лагающим если нет, то формулирующим ошибку?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не знаю. А в чём мотив? Сделать код волума поменьше или что?

unavailableAgent.MutableDevices()->Assign(
timeoutedAgentDevices.begin(),
timeoutedAgentDevices.end());
ExecuteTx<TAddLaggingAgent>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UpdateVolumeConfigInProgress = true ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не, эта булка про обновление конфига, например при ресайзе.


struct TDeviceTimeoutedRequest
{
const ui32 RowIndex;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RowIndex не определяется по DeviceUUID ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно, но запоминать сильно проще чем искать их каждый раз

@komarevtsev-d komarevtsev-d requested a review from drbasic January 16, 2025 09:25
@komarevtsev-d komarevtsev-d added the recheck Add this label to relaunch checks, it will be automatically removed label Jan 16, 2025
@github-actions github-actions bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Jan 16, 2025
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 68ae225.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3769 3768 0 1 0 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blockstore Add this label to run only cloud/blockstore build and tests on PR large-tests Launch large tests for PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants