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

issue-2722: add ability to read from certain replica of mirrored disk #2918

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

Conversation

WilyTiger
Copy link
Collaborator

In order to provide better diagnostics for checksum mismatches in replicas, I want to add the ability for blockstore-client to read certain replica and compare it further in scripts

Copy link
Contributor

Note

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

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
6350 6348 0 2 0 0

@WilyTiger WilyTiger force-pushed the users/wilytiger/issue-2722-3 branch from eb6020e to b0eafc5 Compare January 29, 2025 18:25
Copy link
Contributor

Note

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

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
6303 6301 0 2 0 0

Copy link
Contributor

github-actions bot commented Feb 3, 2025

Note

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

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 655ec6f.

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

@WilyTiger WilyTiger marked this pull request as ready for review February 3, 2025 16:12
@WilyTiger WilyTiger requested review from sharpeye and drbasic February 3, 2025 17:51
Copy link
Contributor

github-actions bot commented Feb 3, 2025

Note

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

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
6306 6298 0 0 6 2

Opts.AddLongOption(
"replica-index",
"from which replica(numerate from 1) read data, only for "
"ssd-io disks")
Copy link
Collaborator

Choose a reason for hiding this comment

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

В blockstore-client есть mirrorX/ssd_mirrorX, про ssd-io он не знает

Comment on lines +432 to +455
if (replicaIndex) {
if (replicaIndex > State.GetReplicaInfos().size()) {
auto response =
std::make_unique<typename TMethod::TResponse>(MakeError(
E_ARGUMENT,
TStringBuilder()
<< "Request " << TMethod::Name
<< " has incorrect ReplicaIndex " << replicaIndex
<< " disk has " << State.GetReplicaInfos().size()
<< " replicas"));
NCloud::Reply(ctx, *ev, std::move(response));
return;
}

if (!replicaActorIds.insert(replicaActorId).second) {
break;
const auto& replicaInfo = State.GetReplicaInfos()[replicaIndex - 1];
if (!replicaInfo.Config->DevicesReadyForReading(blockRange)) {
auto response =
std::make_unique<typename TMethod::TResponse>(MakeError(
E_REJECTED,
TStringBuilder() << "Cannot process " << TMethod::Name
<< " cause replica " << replicaIndex
<< " has not ready devices"));
NCloud::Reply(ctx, *ev, std::move(response));
return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Лучше вынести выбор реплик в отдельную функцию:

auto TMirrorPartitionActor::SelectReplicasToReadFrom(
    ui32 replicaIndex,
    TBlockRange64 blockRange) -> TResultOrError<TVector<TActorId>>
{...}
const auto replicaIndex = record.GetHeaders().GetReplicaIndex();

auto [replicaActorIds, error] =
    SelectReplicasToReadFrom(replicaIndex, blockRange);

if (HasError(error)) {
    NCloud::Reply(ctx, *ev, std::make_unique<TResponse>(error));
    return;
}

if (replicaIndex) {
if (replicaIndex > State.GetReplicaInfos().size()) {
auto response =
std::make_unique<typename TMethod::TResponse>(MakeError(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Удобно сделать using в начале фyнкции:

using TResponse = TMethod::TResponse;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants