test(spanner): support filtering of PickRandomInstance() candidates

[devbww]

Support filtering of PickRandomInstance() candidates (1) on the server
side, by exposing the ListInstances filter, and (2) on the client side,
via a predicate over Instance and InstanceConfig.

This will be needed to support the upcoming CMEK integration tests (at
least in their current configuration), where the selected instance must
be in the single location we use to store the encryption keys.

Also use server-side filtering to implement the restriction of instance
IDs to "test-instance-*".

Fixes #5588.

---

This change is 

[codecov]

Codecov Report

Merging #5885 (397c686) into master (adad709) will decrease coverage by 0.00%.
The diff coverage is 94.73%.

@@            Coverage Diff             @@
##           master    #5885      +/-   ##
==========================================
- Coverage   95.46%   95.46%   -0.01%     
==========================================
  Files        1120     1120              
  Lines      102858   102869      +11     
==========================================
+ Hits        98191    98201      +10     
- Misses       4667     4668       +1     

Impacted Files	Coverage Δ
...ogle/cloud/spanner/testing/pick_random_instance.cc	70.27% <94.73%> (+12.57%)	⬆️
google/cloud/spanner/internal/spanner_stub.cc	76.34% <0.00%> (-2.16%)	⬇️
...ud/spanner/integration_tests/client_stress_test.cc	84.39% <0.00%> (-0.71%)	⬇️
...bigtable/examples/bigtable_hello_instance_admin.cc	84.44% <0.00%> (+2.22%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update adad709...397c686. Read the comment docs.

[coryan]

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @devbww)

---

google/cloud/spanner/testing/pick_random_instance.h, line 39 at r1 (raw file):

    google::cloud::internal::DefaultPRNG& generator,
    std::string const& project_id, std::string const& filter = "",
    std::function<bool(

Consider a type alias, it was hard for me to find the predicate in this case.

---

google/cloud/spanner/testing/pick_random_instance.cc, line 36 at r1 (raw file):

  std::vector<google::spanner::admin::instance::v1::InstanceConfig> configs;
  if (predicate) {

I think if you take predicate by value you can do something like:

if (!predicate) {
  predicate = [](auto, auto) { return true; }
}

And the code below gets simpler. Well, you can't use auto in the lambda, because C++11, but you get the idea.

[devbww]

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @coryan)

---

google/cloud/spanner/testing/pick_random_instance.h, line 39 at r1 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

Consider a type alias, it was hard for me to find the predicate in this case.

Done.

---

google/cloud/spanner/testing/pick_random_instance.cc, line 36 at r1 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

I think if you take predicate by value you can do something like:

if (!predicate) {
  predicate = [](auto, auto) { return true; }
}

And the code below gets simpler. Well, you can't use auto in the lambda, because C++11, but you get the idea.

I was trying to avoid (1) copying the std::function, and (2) fetching the instance configs every time, but this is test code, so done.

[coryan]

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @devbww)

---

google/cloud/spanner/testing/pick_random_instance.cc, line 36 at r1 (raw file):

Previously, devbww (Bradley White) wrote…

I was trying to avoid (1) copying the std::function, and (2) fetching the instance configs every time, but this is test code, so done.

Ack. Thanks for making the change.

---

google/cloud/spanner/testing/pick_random_instance.cc, line 58 at r2 (raw file):

    if (!instance) return std::move(instance).status();
    for (auto const& config : configs) {
      if (instance->config() == config.name()) {

nit: if you wanted to, you can organize the configs into some kind of map or hash as they are loaded and avoid the double loop. But as you said elsewhere, this is test code.

[devbww]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @devbww)

---

google/cloud/spanner/testing/pick_random_instance.cc, line 58 at r2 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

nit: if you wanted to, you can organize the configs into some kind of map or hash as they are loaded and avoid the double loop. But as you said elsewhere, this is test code.

Thanks. But yes ... test code. And I think we might also find that linear search outperforms associative lookup when the number of configs is small (as expected), and particularly so when it is within another loop and cache effects kick in when data is more contiguous.