feat: attemp DirectPath by default by WeiranFang · Pull Request #467 · googleapis/java-bigtable

WeiranFang · 2020-10-14T19:15:31Z

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-endpoint and -Dbigtable.directpath-admin-endpoint to override the default endpoint with DP enabled endpoints.

@igorbernstein2
@mgarolera
@mohanli-ml

codecov · 2020-10-14T19:29:34Z

Codecov Report

Merging #467 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             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

Impacted Files	Coverage Δ	Complexity Δ
...ble/data/v2/stub/EnhancedBigtableStubSettings.java	`98.65% <100.00%> (+1.72%)`	`21.00 <0.00> (-1.00)`	⬆️
...om/google/cloud/bigtable/emulator/v2/Emulator.java	`59.83% <0.00%> (-0.82%)`	`14.00% <0.00%> (ø%)`

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 647f926...d9bfde2. Read the comment docs.

kolea2 · 2020-10-15T14:59:31Z

...table/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java


-    if (DIRECT_PATH_ENDPOINT.equals(builder.getEndpoint())) {
-      logger.warning(
-          "Connecting to Bigtable using DirectPath."


is there anywhere we can keep this logging information to confirm (in tests, for example) that we are connecting using DirectPath?

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 · 2020-10-15T15:01:40Z

...table/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java

-  // TODO(weiranf): Remove this once DirectPath goes to public beta
-  private static boolean isDirectPathEnabled() {
-    return Boolean.getBoolean("bigtable.attempt-directpath");
+        .setAttemptDirectPath(true);


Can you add a comment on why we attemt DirectPath? to give context to future readers.

Added comment. PTAL at the wording, thanks!

igorbernstein2

lgtm

This reverts commit 89c622d.

🤖 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).

…pis#520) This reverts commit 89c622d.

🤖 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).

* feat: attemp DirectPath by default * Add comment for DP attempt

…pis#520) This reverts commit 89c622d.

🤖 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).

feat: attemp DirectPath by default

065fd56

WeiranFang requested a review from a team as a code owner October 14, 2020 19:15

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 14, 2020

product-auto-label bot added the api: bigtable Issues related to the googleapis/java-bigtable API. label Oct 15, 2020

kolea2 reviewed Oct 15, 2020

View reviewed changes

mgarolera reviewed Oct 15, 2020

View reviewed changes

igorbernstein2 approved these changes Oct 15, 2020

View reviewed changes

Add comment for DP attempt

d9bfde2

igorbernstein2 merged commit 89c622d into googleapis:master Oct 19, 2020

janhicken mentioned this pull request Nov 9, 2020

ComputeEngineChannelBuilder fails on App Engine grpc/grpc-java#7604

Closed

WeiranFang added a commit to WeiranFang/java-bigtable that referenced this pull request Nov 10, 2020

Revert "feat: attemp DirectPath by default (googleapis#467)"

1abfa37

This reverts commit 89c622d.

WeiranFang mentioned this pull request Nov 10, 2020

Revert "feat: attemp DirectPath by default" #520

Merged

kolea2 pushed a commit that referenced this pull request Nov 10, 2020

Revert "feat: attemp DirectPath by default (#467)" (#520)

b33b0fc

This reverts commit 89c622d.

release-please bot mentioned this pull request Nov 10, 2020

chore: release 1.17.2 #519

Merged

mutianf pushed a commit to mutianf/java-bigtable that referenced this pull request Nov 12, 2020

Revert "feat: attemp DirectPath by default (googleapis#467)" (googlea…

d0cb5cf

…pis#520) This reverts commit 89c622d.

mohanli-ml mentioned this pull request Nov 12, 2020

feat(bigtable): run E2E test over DirectPath googleapis/google-cloud-go#3116

Merged

mohanli-ml mentioned this pull request Nov 30, 2020

feat: attempt DirectPath by default #544

Merged

ad548 pushed a commit to ad548/java-bigtable that referenced this pull request Mar 13, 2021

feat: attemp DirectPath by default (googleapis#467)

c56643a

* 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

Revert "feat: attemp DirectPath by default (googleapis#467)" (googlea…

658e698

…pis#520) This reverts commit 89c622d.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: attemp DirectPath by default#467

feat: attemp DirectPath by default#467
igorbernstein2 merged 2 commits intogoogleapis:masterfrom
WeiranFang:dp-default

WeiranFang commented Oct 14, 2020

Uh oh!

codecov bot commented Oct 14, 2020 •

edited

Loading

Uh oh!

kolea2 Oct 15, 2020

Uh oh!

igorbernstein2 Oct 15, 2020

Uh oh!

mgarolera Oct 15, 2020

Uh oh!

WeiranFang Oct 15, 2020

Uh oh!

igorbernstein2 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

WeiranFang commented Oct 14, 2020

Uh oh!

codecov bot commented Oct 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kolea2 Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

igorbernstein2 Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

mgarolera Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

WeiranFang Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

igorbernstein2 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Oct 14, 2020 •

edited

Loading