Skip to content

Commit

Permalink
fix(spanner): add Options save/restore to PartialResultSetSource
Browse files Browse the repository at this point in the history
Save the options in force during `PartialResultSetSource` construction,
and then restore them over `PartialResultSetReader` calls.  This means
that `NextRow()`, `Finish()` and `TryCancel()` will have the right
options installed during any RPCs they execute.

Tighten up the test by installing a non-matching `OptionsSpan` before
any `NextRow()` calls are made, and by expecting no `TryCancel()`s in
cases that complete their enumeration.

This is the Spanner equivalent of #8256.
  • Loading branch information
devbww committed Feb 12, 2022
1 parent a5a5458 commit 1c104a1
Show file tree
Hide file tree
Showing 4 changed files with 375 additions and 87 deletions.
2 changes: 2 additions & 0 deletions google/cloud/spanner/internal/partial_result_set_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ PartialResultSetSource::~PartialResultSetSource() {
// If there is actual data in the streaming RPC Finish() can deadlock, so
// before trying to read the final status we need to cancel the streaming
// RPC.
internal::OptionsSpan span(options_);
reader_->TryCancel();
// The user didn't iterate over all the data; finish the stream on their
// behalf, but we have no way to communicate error status.
Expand All @@ -95,6 +96,7 @@ PartialResultSetSource::~PartialResultSetSource() {
}

Status PartialResultSetSource::ReadFromStream() {
internal::OptionsSpan span(options_);
auto result_set = reader_->Read();
if (!result_set) {
// Read() returns false for end of stream, whether we read all the data or
Expand Down
4 changes: 3 additions & 1 deletion google/cloud/spanner/internal/partial_result_set_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "google/cloud/spanner/results.h"
#include "google/cloud/spanner/value.h"
#include "google/cloud/spanner/version.h"
#include "google/cloud/options.h"
#include "google/cloud/status.h"
#include "google/cloud/status_or.h"
#include "absl/types/optional.h"
Expand Down Expand Up @@ -60,10 +61,11 @@ class PartialResultSetSource : public ResultSourceInterface {
private:
explicit PartialResultSetSource(
std::unique_ptr<PartialResultSetReader> reader)
: reader_(std::move(reader)) {}
: options_(internal::CurrentOptions()), reader_(std::move(reader)) {}

Status ReadFromStream();

Options options_;
std::unique_ptr<PartialResultSetReader> reader_;
absl::optional<google::spanner::v1::ResultSetMetadata> metadata_;
absl::optional<google::spanner::v1::ResultSetStats> stats_;
Expand Down
Loading

0 comments on commit 1c104a1

Please sign in to comment.