chore: add DirectPath fallback integration test #675
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDirectPathFallback.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDirectPathFallback.java
Outdated
Show resolved
Hide resolved
9c44cfc to
e04a5a5
Compare
thiagotnunes
reviewed
Dec 1, 2020
| // Create a new testHelper with the instrumented transport provider | ||
| try { | ||
| testHelper = RemoteSpannerHelper.create(builder.build(), env.getTestHelper().getInstanceId()); | ||
| } catch (Throwable t) { |
Contributor
There was a problem hiding this comment.
Could we print a message / exit the test if something goes wrong here? Also, could we catch only Exception instead of Throwable?
Contributor
Author
There was a problem hiding this comment.
The original function https://github.com/googleapis/java-spanner/blob/master/google-cloud-spanner/src/main/java/com/google/cloud/spanner/testing/RemoteSpannerHelper.java#L158 throws a Throwable. Since we do not want to catch a Throwable, I directly throw it.
|
|
||
| boolean seenEnough = false; | ||
|
|
||
| while (!seenEnough && stopwatch.elapsed(TimeUnit.MINUTES) < 2) { |
Contributor
There was a problem hiding this comment.
Do we have to wait for 2 minutes for this test? This slows down the whole integration suite when running locally.
Contributor
Author
There was a problem hiding this comment.
The while loop will wait at most 2 minutes to get directpath traffic. In my manual test, the client should be able to get directpath traffic within a few seconds, then seenEnough becomes true, and the while loop will stop.
| * Test DirectPath fallback behavior by injecting a ChannelHandler into the netty stack that will | ||
| * disrupt IPv6 communications. | ||
| * | ||
| * <p>WARNING: this test can only be run on a GCE VM and will explicitly ignore |
Contributor
There was a problem hiding this comment.
Does this test pass locally? Will be users be able to run these (if not, maybe we should put them under the direct path profile, which is not enabled by default)?
Contributor
Author
There was a problem hiding this comment.
Yes, my local test command mvn -Penable-integration-tests,spanner-directpath-it -DskipUTs=true -DtrimStackTrace=false -Dclirr.skip=true -Denforcer.skip=true -Dit.test=ITDirectPathFallback -DfailIfNoTests=false -Dspanner.directpath_test_scenario=ipv6 -fae verify can pass. I think users should not run this test since users are not supposed to know if the traffic is CFE or DirectPath. And to make sure the test is being running under direct path profile, I added a check at the beginning of the setup() function.
Codecov Report
@@ Coverage Diff @@
## master #675 +/- ##
============================================
+ Coverage 84.07% 84.12% +0.04%
- Complexity 2496 2510 +14
============================================
Files 141 142 +1
Lines 13812 13911 +99
Branches 1317 1323 +6
============================================
+ Hits 11613 11703 +90
- Misses 1654 1662 +8
- Partials 545 546 +1
Continue to review full report at Codecov.
|
olavloite
reviewed
Dec 1, 2020
| @@ -0,0 +1,305 @@ | |||
| /* | |||
| * Copyright 2017 Google LLC | |||
| private static Database db; | ||
| private static DatabaseClient client; | ||
|
|
||
| private static final String DIRECT_PATH_ENDPOINT = "aa423245250f2bbf.sandbox.googleapis.com:443"; |
Collaborator
There was a problem hiding this comment.
I assume that this is temporary and will be replaced by a environment variable or similar at a later moment?
Contributor
Author
There was a problem hiding this comment.
Yes this is temporary. It will be removed once DirectPath goes to public beta, and this test will use the default Spanner endpoint. I added a TODO here.
| + " Key STRING(MAX) NOT NULL," | ||
| + " StringValue STRING(MAX)," | ||
| + ") PRIMARY KEY (Key)", | ||
| "CREATE INDEX TestTableByValue ON TestTable(StringValue)", |
Collaborator
There was a problem hiding this comment.
nit: Could we remove these indexes? I don't think they are relevant for this test, and creating and maintaining secondary indexes are expensive operations.
olavloite
approved these changes
Dec 2, 2020
olavloite
reviewed
Dec 16, 2020
| <groupId>io.grpc</groupId> | ||
| <artifactId>grpc-protobuf</artifactId> | ||
| </dependency> | ||
| <dependency> |
Collaborator
There was a problem hiding this comment.
It seems that this change is now causing this problem further down the line with the flatten-maven-plugin: googleapis/java-spanner-jdbc#293 (comment)
thiagotnunes
pushed a commit
that referenced
this pull request
May 6, 2021
ansh0l
pushed a commit
to ansh0l/java-spanner
that referenced
this pull request
Nov 10, 2022
This is an auto-generated regeneration of the .pb.go files by cloud.google.com/go/internal/gapicgen. Once this PR is submitted, genbot will update the corresponding PR to depend on the newer version of go-genproto, and assign reviewers. Whilst this or any regen PR is open in go-genproto, genbot will not create any more regeneration PRs. If all regen PRs are closed, gapicgen will create a new set of regeneration PRs once per night. If you have been assigned to review this PR, please: - Ensure that CI is passing. If it's failing, it requires your manual attention. - Approve and submit this PR if you believe it's ready to ship. That will prompt genbot to assign reviewers to the google-cloud-go PR. Corresponding google-cloud-go PR: googleapis/google-cloud-go#4757 Changes: feat(functions): add SecurityLevel option on HttpsTrigger PiperOrigin-RevId: 396889803 Source-Link: googleapis/googleapis@44c497f chore: regenerate API index Source-Link: googleapis/googleapis@231af01 feat: IdentityToolkit v2 public protos Committer: @alexander-fenster PiperOrigin-RevId: 396886667 Source-Link: googleapis/googleapis@a9aafd5 docs(bigquery/storage): Align session length with public documentation feat: Expose estimated bytes that a session will scan. Committer: @emkornfield PiperOrigin-RevId: 396849937 Source-Link: googleapis/googleapis@5661452 chore: regenerate API index Source-Link: googleapis/googleapis@a27fb7a feat: added a flag to enable/disable gvnic on a node pool feat: added node pool level network config feat: added update support for node pool labels, taints and network tags feat: added configuration for workload certificates and identity service component feat: added configuration for node pool defaults, autopilot, logging and monitoring feat: added the option to specify L4 load balancer configuration and IP v6 configuration feat: added the option to list supported windows versions fix: deprecated KALM addon config option fix: deprecated cluster status condition code docs: clarified SetNodePoolSize API behavior PiperOrigin-RevId: 396762326 Source-Link: googleapis/googleapis@1f1d09d feat(talent): Added a new `KeywordMatchMode` field to support more keyword matching options feat: Added more `DiversificationLevel` configuration options PiperOrigin-RevId: 396667150 Source-Link: googleapis/googleapis@caad48d build(nodejs): correct artifact name for npm PiperOrigin-RevId: 396640130 Source-Link: googleapis/googleapis@c53262d chore(accessapproval): remove nonexistent method from config PiperOrigin-RevId: 396610698 Source-Link: googleapis/googleapis@bdde047 chore: Kokoro CI configuration PiperOrigin-RevId: 396477636 Source-Link: googleapis/googleapis@067740a chore: Remove unnecessary double space in example API PiperOrigin-RevId: 396382685 Source-Link: googleapis/googleapis@885183f Synchronize new proto/yaml changes. PiperOrigin-RevId: 396379148 Source-Link: googleapis/googleapis@de0d477
ansh0l
pushed a commit
to ansh0l/java-spanner
that referenced
this pull request
Nov 10, 2022
This is an auto-generated regeneration of the .pb.go files by cloud.google.com/go/internal/gapicgen. Once this PR is submitted, genbot will update the corresponding PR to depend on the newer version of go-genproto, and assign reviewers. Whilst this or any regen PR is open in go-genproto, genbot will not create any more regeneration PRs. If all regen PRs are closed, gapicgen will create a new set of regeneration PRs once per night. If you have been assigned to review this PR, please: - Ensure that CI is passing. If it's failing, it requires your manual attention. - Approve and submit this PR if you believe it's ready to ship. That will prompt genbot to assign reviewers to the google-cloud-go PR. Corresponding google-cloud-go PR: googleapis/google-cloud-go#5112 Changes: feat(texttospeech): update v1beta1 proto PiperOrigin-RevId: 409139454 Source-Link: googleapis/googleapis@7c10623 chore: regenerate API index Source-Link: googleapis/googleapis@753ad85 chore(compute): Add C# generation for Compute v1 Source-Link: googleapis/googleapis@05a6a82 feat(dialogflow/cx): allow setting custom CA for generic webhooks PiperOrigin-RevId: 408995680 Source-Link: googleapis/googleapis@76f7f48 fix!(networkconnectivity): Mark a couple networkconnectivity API fields as required, to match implemented behavior PiperOrigin-RevId: 408969147 Source-Link: googleapis/googleapis@cc003a4 chore(aiplatform): Add partial C# generation to AI Platform PiperOrigin-RevId: 408964828 Source-Link: googleapis/googleapis@895ad28 chore(binaryauthorization): Fix SystemPolicy service name for BinaryAuthorization V1beta1 PiperOrigin-RevId: 408764899 Source-Link: googleapis/googleapis@66878ca chore(iam/credentials): Update iam/credentials BUILD.bazel package name to google-cloud-iam chore(python): Update iam/credentials namespace to google.cloud.iam_credentials PiperOrigin-RevId: 408717007 Source-Link: googleapis/googleapis@171d777 docs: fix docstring formatting Committer: @parthea PiperOrigin-RevId: 408715661 Source-Link: googleapis/googleapis@5bb0518 chore(container): Update container BUILD.bazel package name to google-cloud-container chore(python): Update namespace for container API to google.cloud PiperOrigin-RevId: 408673815 Source-Link: googleapis/googleapis@c08b149 chore: Add C# generation rules PiperOrigin-RevId: 408640431 Source-Link: googleapis/googleapis@b32a324 docs(datacatalog): Improved formatting PiperOrigin-RevId: 408569512 Source-Link: googleapis/googleapis@c7b3bd0 docs(securitycenter): fix docstring formatting Committer: @parthea PiperOrigin-RevId: 408564402 Source-Link: googleapis/googleapis@3020af5 feat(compute): Move compute.v1 from googleapis-discovery to googleapis (googleapis#675) * feat: Move compute.v1 from googleapis-discovery to googleapis The WORKSPACE file changes are expected to be overridden by the next sync, and it is ok, as the synce'd copy will contain the same changes. * feat: sync latest compute.proto files from googleapis-discovery Source-Link: googleapis/googleapis@691a18b docs(samples): add example tags to generated samples PiperOrigin-RevId: 408439482 Source-Link: googleapis/googleapis@b9f6184 chore(aiplatform): update Java and Python dependencies PiperOrigin-RevId: 408420890 Source-Link: googleapis/googleapis@2921f9f feat: update gapic-generator-csharp to 1.3.15 feat: update rules_gapic to 0.10.0 Committer: @viacheslav-rostovtsev PiperOrigin-RevId: 408407783 Source-Link: googleapis/googleapis@1bd869b docs(gaming): fix docstring formatting Committer: @parthea PiperOrigin-RevId: 408383287 Source-Link: googleapis/googleapis@2aec4d0 feat(securitycenter): Added resource type and display_name field to the FindingResult, and supported them in the filter for ListFindings and GroupFindings. Also added display_name to the resource which is surfaced in NotificationMessage PiperOrigin-RevId: 408362247 Source-Link: googleapis/googleapis@4d71c45 feat(redis): [Cloud Memorystore for Redis] Support Multiple Read Replicas when creating Instance PiperOrigin-RevId: 408360324 Source-Link: googleapis/googleapis@78eb8a2 feat(redis): [Cloud Memorystore for Redis] Support Multiple Read Replicas when creating Instance PiperOrigin-RevId: 408360267 Source-Link: googleapis/googleapis@8625cf0 docs(storage/internal): Add comments to GCS gRPC API proto spec to describe how naming works PiperOrigin-RevId: 408352508 Source-Link: googleapis/googleapis@be4be3d chore: regenerate API index Source-Link: googleapis/googleapis@9974d09 feat(binaryauthorization): add new admission rule types to Policy feat: update SignatureAlgorithm enum to match algorithm names in KMS feat: add SystemPolicyV1Beta1 service PiperOrigin-RevId: 408346628 Source-Link: googleapis/googleapis@3dfbdc3 feat(datacatalog): Added BigQueryDateShardedSpec.latest_shard_resource field feat: Added SearchCatalogResult.display_name field feat: Added SearchCatalogResult.description field PiperOrigin-RevId: 408274419 Source-Link: googleapis/googleapis@cbba92c docs(resourcemanager): fix docstring formatting Committer: @parthea PiperOrigin-RevId: 407854413 Source-Link: googleapis/googleapis@f81b7bd chore: try to fix the mistaken-pull-closer config PiperOrigin-RevId: 407849101 Source-Link: googleapis/googleapis@9e6c8c6 docs(ruby): Add simple code examples for all RPC methods PiperOrigin-RevId: 407847849 Source-Link: googleapis/googleapis@1112610 feat(workflows/executions): add a stack_trace field to the Error messages specifying where the error occured feat: add call_log_level field to Execution messages doc: clarify requirement to escape strings within JSON arguments Update the Execution proto with stack_trace field which is populated on output if an error occurs, and the call_log_level field can be specified on input to request that function calls and/or errors for the given execution be logged to Cloud Logging. PiperOrigin-RevId: 407842136 Source-Link: googleapis/googleapis@cd48c16 feat(dialogflow): added support to configure security settings, language code and time zone on conversation profile PiperOrigin-RevId: 407663596 Source-Link: googleapis/googleapis@f9acb37 chore: generate java files for apps/script/type protos PiperOrigin-RevId: 407655547 Source-Link: googleapis/googleapis@5946b7a feat(functions): Secret Manager integration fields 'secret_environment_variables' and 'secret_volumes' added feat: CMEK integration fields 'kms_key_name' and 'docker_repository' added PiperOrigin-RevId: 407654258 Source-Link: googleapis/googleapis@09b2576 chore: regenerate API index Source-Link: googleapis/googleapis@ddd6637 feat(contactcenterinsights): Add ability to update phrase matchers feat: Add issue model stats to time series feat: Add display name to issue model stats PiperOrigin-RevId: 407647313 Source-Link: googleapis/googleapis@33fd29d
rajatbhatta
pushed a commit
to rajatbhatta/java-spanner
that referenced
this pull request
Nov 17, 2022
🤖 I have created a release \*beep\* \*boop\* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a DirectPath fallback test for spanner. The test is basically the same as what we have for bigtable (https://github.com/googleapis/java-bigtable/blob/master/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/DirectPathFallbackIT.java). The logic of the test is:
The DirectPath and CFE traffic are distinguished by peer IP.