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

Do not raise crit event when agent is not found #2560

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

komarevtsev-d
Copy link
Collaborator

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

Недавно добавленный DiskRegistryCleanupAgentConfigError начал загораться. Для этого достаточно позвать два раза REMOVE_HOST

@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 24, 2024
Copy link
Contributor

Note

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

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 9d6bcd0.

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

@@ -5537,7 +5537,7 @@ NProto::TError TDiskRegistryState::TryToRemoveAgentDevices(
[&](const auto& x) { return x.GetAgentId() == agentId; });
if (agentIt == agents->end()) {
return MakeError(
E_NOT_FOUND,
S_FALSE,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Был бы это RemoveAgent, было бы ОК, но логичнее будет проверять E_NOT_FOUND снаружи - в CleanupAgentConfig:

auto error = TryToRemoveAgentDevices(db, agent.GetAgentId());
if (!HasError(error) || error.GetCode() == E_NOT_FOUND) {
    return;
}

@komarevtsev-d komarevtsev-d force-pushed the users/komarevtsev-d/fix-cleanup-agent-config/0 branch from 9d6bcd0 to 22908d3 Compare November 25, 2024 08:46
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