storage: ApplyWriteRedirectErrors clears the write handle when a BidiWriteObjectRedirectedError omits it

Aberta Para iniciantes
  • Algo errado?
#16,529 0 comentários 0 reações 0 responsáveis Ver no GitHub

Mantenedores costumam responder em até 1 dia

Ninguém assumiu esta issue ainda.

Avaliação

Dificuldade
2/5
Tempo estimado
1-3 horas
Facilidade para iniciantes
82/100
Tipo de issue
Bug
Clareza
Claramente especificada
Status de atividade
Ativa
Stack de tecnologia
cpp
Domínio
cloud

Direção de pesquisa

Comece em google/cloud/storage/internal/async/handle_redirect_error.cc e compare ApplyWriteRedirectErrors com HandleBidiWriteRedirect; depois, leia Resume() em google/cloud/storage/internal/async/writer_connection_resumed.cc. Está concluído quando um redirecionamento sem um write_handle preserva o handle existente, enquanto os redirecionamentos que fornecem um continuam aplicando-o.

Escrita pelo modelo de indexação a partir do texto da issue.

Descrição

api: storage priority: p2 type: bug

Does this issue affect the google-cloud-cpp project? If the problem is with
the Google Cloud service exposed by the google-cloud-cpp libraries instead of
the client libraries themselves, you may consider opening a support request
instead. The google-cloud-cpp developers cannot help you troubleshoot problems
with the service itself.

Yes

What component of google-cloud-cpp is this related to? For example, is
this related to bigtable (i.e., something in google/cloud/bigtable), or GCS
(i.e., something in google/cloud/storage)?

google/cloud/storage

Describe the bug A clear and concise description of what the bug is.

Accidentally cleared write handle

To Reproduce Steps to reproduce the behavior:

Found through static analysis of the code

Expected behavior A clear and concise description of what you expected to
happen.

Don't clear the write handle

Operating system: If you are using a Linux distribution please include the
name and version of the distribution too.

N/A

What compiler and version are you using? Please include the output of
g++ -v or clang++ -v or the equivalent command-line flag.

N/A

What version of google-cloud-cpp are you using? Please include the output
from git rev-parse HEAD if you are compiling from source, or the version
number from the applicable google/cloud/*/version.h file.

Found by examining most recent version. 10/2/26.

Additional context Add any other context about the problem here.

When an appendable bidi write is redirected mid-upload, ApplyWriteRedirectErrors copies write_handle from the redirect without checking whether it is set:

// google/cloud/storage/internal/async/handle_redirect_error.cc
void ApplyWriteRedirectErrors(google::storage::v2::AppendObjectSpec& spec,
                              google::rpc::Status const& rpc_status) {
  for (auto const& any : rpc_status.details()) {
    ...
    *spec.mutable_write_handle() = std::move(*error.mutable_write_handle());   // unconditional
    *spec.mutable_routing_token() = std::move(*error.mutable_routing_token()); // unconditional
    if (error.has_generation()) spec.set_generation(error.generation());       // guarded
  }
}

BidiWriteObjectRedirectedError.write_handle is optional. The proto says that if it is not set, "clients might retry the original request" (google/storage/v2/storage.proto). When the server omits it, the code above replaces the handle that AsyncWriterConnectionResumedState::Resume() just set from latest_write_handle_ with an empty one, so the resume request carries a write_handle that is set but empty:

// google/cloud/storage/internal/async/writer_connection_resumed.cc, Resume()
// Include write_handle to enable fast resume instead of slow
// takeover. Without handle, server performs full state validation.
if (latest_write_handle_) {
  *append_object_spec.mutable_write_handle() = *latest_write_handle_;
}
append_object_spec.set_generation(first_response_.resource().generation());
ApplyWriteRedirectErrors(append_object_spec, std::move(proto_status));

This is inconsistent with the rest of the SDK:

  • generation on the next line is only overridden when the redirect sets it.
  • HandleBidiWriteRedirect (same file, used when the stream is first opened) leaves the request unchanged when the redirect has no write_handle.

Impact: this isn't a correctness problem, since the resume still uses an AppendObjectSpec with the right generation. But per the comment in Resume(), resuming without a valid handle makes the server do a slow takeover with full state validation instead of a fast resume. This may be worth investigating for write performance after redirects. We haven't checked how the server treats a handle that is set but empty, as opposed to a missing one.

We hit token-only redirects in practice: a reconnect after a RESOURCE_EXHAUSTED stream failure received ABORTED: The object is not available from this location. with a BidiWriteObjectRedirectedError that had only a routing_token.

Suggested fix: guard the handle the same way as the generation:

if (error.has_write_handle()) {
  *spec.mutable_write_handle() = std::move(*error.mutable_write_handle());
}

(Optionally also only override routing_token when it is non-empty, to match HandleBidiWriteRedirect. Note that routing tokens may expire, so keeping an old one has tradeoffs.)

Linguagem predominante
C++
Estrelas
660
Forks
479
Merge médio
1d 10h
PRs com merge (30d)
65

Preparar o ambiente

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Mais de googleapis/google-cloud-cpp

Todas as issues de googleapis/google-cloud-cpp

Issues semelhantes

Mais issues de C++

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.