feat: attemp DirectPath by default #467
Merged
Conversation
This file contains 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-cla
bot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #467 +/- ##
============================================
+ Coverage 80.65% 80.68% +0.02%
+ Complexity 1115 1114 -1
============================================
Files 105 105
Lines 6941 6936 -5
Branches 369 367 -2
============================================
- Hits 5598 5596 -2
Misses 1144 1144
+ Partials 199 196 -3
Continue to review full report at Codecov.
|
product-auto-label
bot
added
the
api: bigtable
kolea2
reviewed
Oct 15, 2020
| @@ -166,12 +163,6 @@ | |||
| private EnhancedBigtableStubSettings(Builder builder) { | |||
| super(builder); | |||
|
|
|||
| if (DIRECT_PATH_ENDPOINT.equals(builder.getEndpoint())) { | |||
| logger.warning( | |||
| "Connecting to Bigtable using DirectPath." | |||
There was a problem hiding this comment.
is there anywhere we can keep this logging information to confirm (in tests, for example) that we are connecting using DirectPath?
There was a problem hiding this comment.
For tests we inject hooks into netty to ensure that directpath is used, also the integration tests generate verbose grpc logs which would show directpath usage. For prod usage, we dont really have a good way to introspect it as its a grpc level feature
mgarolera
reviewed
Oct 15, 2020
| // TODO(weiranf): Remove this once DirectPath goes to public beta | ||
| private static boolean isDirectPathEnabled() { | ||
| return Boolean.getBoolean("bigtable.attempt-directpath"); | ||
| .setAttemptDirectPath(true); |
There was a problem hiding this comment.
Can you add a comment on why we attemt DirectPath? to give context to future readers.
There was a problem hiding this comment.
Added comment. PTAL at the wording, thanks!
gcf-merge-on-green bot
pushed a commit
that referenced
this pull request
Oct 23, 2020
🤖 I have created a release \*beep\* \*boop\* --- ## [1.17.0](https://www.github.com/googleapis/java-bigtable/compare/v1.16.2...v1.17.0) (2020-10-23) ### Features * attemp DirectPath by default ([#467](https://www.github.com/googleapis/java-bigtable/issues/467)) ([89c622d](https://www.github.com/googleapis/java-bigtable/commit/89c622da6038067892687af3edafae743465eda7)) * backup level IAM ([#450](https://www.github.com/googleapis/java-bigtable/issues/450)) ([f38a8ec](https://www.github.com/googleapis/java-bigtable/commit/f38a8ecdc6164d081ef96f748ea37bd62b29b419)) * Implement toString for Bigtable*Settings ([#488](https://www.github.com/googleapis/java-bigtable/issues/488)) ([4d821f8](https://www.github.com/googleapis/java-bigtable/commit/4d821f85ceb237c8e449243ff8c80fb94e22ad51)) ### Bug Fixes * Make refreshing channel compatible with BigtableDataClientFactory ([#474](https://www.github.com/googleapis/java-bigtable/issues/474)) ([fc74164](https://www.github.com/googleapis/java-bigtable/commit/fc741645536e01fac772136bc8346f73ff95e600)) ### Documentation * fix formatting ([#476](https://www.github.com/googleapis/java-bigtable/issues/476)) ([eb24569](https://www.github.com/googleapis/java-bigtable/commit/eb24569e53f9d2b7fde50748c840c2c11f3f3c80)) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.12.1 ([#475](https://www.github.com/googleapis/java-bigtable/issues/475)) ([9e56edf](https://www.github.com/googleapis/java-bigtable/commit/9e56edfa7b0a78f14518a99130a7b319c5873be4)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.13.0 ([#484](https://www.github.com/googleapis/java-bigtable/issues/484)) ([aad648f](https://www.github.com/googleapis/java-bigtable/commit/aad648fec16b122092d394350822da742a2d7aa0)) * update dependency com.google.truth:truth to v1.1 ([#483](https://www.github.com/googleapis/java-bigtable/issues/483)) ([cca1e0e](https://www.github.com/googleapis/java-bigtable/commit/cca1e0e16f2ec0cc887d81c1844f5395ce08b6ea)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
WeiranFang
added a commit
to WeiranFang/java-bigtable
that referenced
this pull request
Nov 10, 2020
This reverts commit 89c622d.
kolea2
pushed a commit
that referenced
this pull request
Nov 10, 2020
Merged
gcf-merge-on-green bot
pushed a commit
that referenced
this pull request
Nov 10, 2020
🤖 I have created a release \*beep\* \*boop\* --- ### [1.17.2](https://www.github.com/googleapis/java-bigtable/compare/v1.17.1...v1.17.2) (2020-11-10) ### Bug Fixes * readRowSettings use manual readRows settings instead of gapics ([#494](https://www.github.com/googleapis/java-bigtable/issues/494)) ([0ef7c5d](https://www.github.com/googleapis/java-bigtable/commit/0ef7c5d4aacacd2030a52efc148ead457719a927)) ### Reverts * Revert "feat: attemp DirectPath by default (#467)" (#520) ([b33b0fc](https://www.github.com/googleapis/java-bigtable/commit/b33b0fc1b5478934298db317c1168c1e9dc20599)), closes [#467](https://www.github.com/googleapis/java-bigtable/issues/467) [#520](https://www.github.com/googleapis/java-bigtable/issues/520) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
mutianf
pushed a commit
to mutianf/java-bigtable
that referenced
this pull request
Nov 12, 2020
🤖 I have created a release \*beep\* \*boop\* --- ### [1.17.2](https://www.github.com/googleapis/java-bigtable/compare/v1.17.1...v1.17.2) (2020-11-10) ### Bug Fixes * readRowSettings use manual readRows settings instead of gapics ([googleapis#494](https://www.github.com/googleapis/java-bigtable/issues/494)) ([0ef7c5d](https://www.github.com/googleapis/java-bigtable/commit/0ef7c5d4aacacd2030a52efc148ead457719a927)) ### Reverts * Revert "feat: attemp DirectPath by default (googleapis#467)" (googleapis#520) ([b33b0fc](https://www.github.com/googleapis/java-bigtable/commit/b33b0fc1b5478934298db317c1168c1e9dc20599)), closes [googleapis#467](https://www.github.com/googleapis/java-bigtable/issues/467) [googleapis#520](https://www.github.com/googleapis/java-bigtable/issues/520) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
ad548
pushed a commit
to ad548/java-bigtable
that referenced
this pull request
Mar 13, 2021
* feat: attemp DirectPath by default * Add comment for DP attempt
ad548
pushed a commit
to ad548/java-bigtable
that referenced
this pull request
Mar 13, 2021
🤖 I have created a release \*beep\* \*boop\* --- ## [1.17.0](https://www.github.com/googleapis/java-bigtable/compare/v1.16.2...v1.17.0) (2020-10-23) ### Features * attemp DirectPath by default ([googleapis#467](https://www.github.com/googleapis/java-bigtable/issues/467)) ([89c622d](https://www.github.com/googleapis/java-bigtable/commit/89c622da6038067892687af3edafae743465eda7)) * backup level IAM ([googleapis#450](https://www.github.com/googleapis/java-bigtable/issues/450)) ([f38a8ec](https://www.github.com/googleapis/java-bigtable/commit/f38a8ecdc6164d081ef96f748ea37bd62b29b419)) * Implement toString for Bigtable*Settings ([googleapis#488](https://www.github.com/googleapis/java-bigtable/issues/488)) ([4d821f8](https://www.github.com/googleapis/java-bigtable/commit/4d821f85ceb237c8e449243ff8c80fb94e22ad51)) ### Bug Fixes * Make refreshing channel compatible with BigtableDataClientFactory ([googleapis#474](https://www.github.com/googleapis/java-bigtable/issues/474)) ([fc74164](https://www.github.com/googleapis/java-bigtable/commit/fc741645536e01fac772136bc8346f73ff95e600)) ### Documentation * fix formatting ([googleapis#476](https://www.github.com/googleapis/java-bigtable/issues/476)) ([eb24569](https://www.github.com/googleapis/java-bigtable/commit/eb24569e53f9d2b7fde50748c840c2c11f3f3c80)) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.12.1 ([googleapis#475](https://www.github.com/googleapis/java-bigtable/issues/475)) ([9e56edf](https://www.github.com/googleapis/java-bigtable/commit/9e56edfa7b0a78f14518a99130a7b319c5873be4)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.13.0 ([googleapis#484](https://www.github.com/googleapis/java-bigtable/issues/484)) ([aad648f](https://www.github.com/googleapis/java-bigtable/commit/aad648fec16b122092d394350822da742a2d7aa0)) * update dependency com.google.truth:truth to v1.1 ([googleapis#483](https://www.github.com/googleapis/java-bigtable/issues/483)) ([cca1e0e](https://www.github.com/googleapis/java-bigtable/commit/cca1e0e16f2ec0cc887d81c1844f5395ce08b6ea)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
ad548
pushed a commit
to ad548/java-bigtable
that referenced
this pull request
Mar 13, 2021
🤖 I have created a release \*beep\* \*boop\* --- ### [1.17.2](https://www.github.com/googleapis/java-bigtable/compare/v1.17.1...v1.17.2) (2020-11-10) ### Bug Fixes * readRowSettings use manual readRows settings instead of gapics ([googleapis#494](https://www.github.com/googleapis/java-bigtable/issues/494)) ([0ef7c5d](https://www.github.com/googleapis/java-bigtable/commit/0ef7c5d4aacacd2030a52efc148ead457719a927)) ### Reverts * Revert "feat: attemp DirectPath by default (googleapis#467)" (googleapis#520) ([b33b0fc](https://www.github.com/googleapis/java-bigtable/commit/b33b0fc1b5478934298db317c1168c1e9dc20599)), closes [googleapis#467](https://www.github.com/googleapis/java-bigtable/issues/467) [googleapis#520](https://www.github.com/googleapis/java-bigtable/issues/520) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Update client to attempt DirectPath by default, and remove DirectPath test endpoint.
Note that it doesn't mean that after this change client will just use DirectPath, but will call the DirectPath codepath by default.
The actually enablement of DirectPath is controlled by service owner via ACL config. For now, after this change, although all users will attempt DirectPath, but they will all just fallback to the original CFE path.
For testing, we will just use
-Dbigtable.directpath-data-endpointand-Dbigtable.directpath-admin-endpointto override the default endpoint with DP enabled endpoints.@igorbernstein2
@mgarolera
@mohanli-ml