Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make keepalive params a bit conservative #528

Merged
merged 1 commit into from Nov 13, 2020

Conversation

sushanb
Copy link
Contributor

@sushanb sushanb commented Nov 11, 2020

Having keepalive pings at 10s can cause too_many_ping commands and it will double the keepalive time for new connections on that channel.

This PR sets keepalive pings at 30s (allowed by Google Frontends) which will avoid too many pings warnings.

Do not set keepAliveWithoutCalls(true) as enabling keepalive on idle streams can increase load.

@sushanb sushanb requested a review from a team as a code owner November 11, 2020 20:29
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 11, 2020
@sushanb
Copy link
Contributor Author

sushanb commented Nov 11, 2020

@ejona86 and @igorbernstein2 FYI

@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/java-bigtable API. label Nov 11, 2020
@codecov
Copy link

codecov bot commented Nov 11, 2020

Codecov Report

Merging #528 (8ea38ce) into master (682d06e) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #528      +/-   ##
============================================
- Coverage     81.28%   81.27%   -0.01%     
  Complexity     1129     1129              
============================================
  Files           106      106              
  Lines          7047     7045       -2     
  Branches        370      370              
============================================
- Hits           5728     5726       -2     
  Misses         1119     1119              
  Partials        200      200              
Impacted Files Coverage Δ Complexity Δ
...e/cloud/bigtable/data/v2/BigtableDataSettings.java 71.42% <100.00%> (-0.41%) 8.00 <0.00> (ø)
...ble/data/v2/stub/EnhancedBigtableStubSettings.java 96.06% <100.00%> (-0.02%) 23.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 682d06e...8ea38ce. Read the comment docs.

@kolea2 kolea2 merged commit f5f66c9 into googleapis:master Nov 13, 2020
@sushanb sushanb deleted the keepalive-conservatibe branch November 13, 2020 19:20
gcf-merge-on-green bot pushed a commit that referenced this pull request Nov 16, 2020
🤖 I have created a release \*beep\* \*boop\* 
---
## [1.18.0](https://www.github.com/googleapis/java-bigtable/compare/v1.17.3...v1.18.0) (2020-11-13)


### Features

* **autogenerated:** Update BigtableTableAdmin GetIamPolicy to include the additional binding for Backup, Change DeleteAppProfileRequest.ignore_warnings to REQUIRED. ([#530](https://www.github.com/googleapis/java-bigtable/issues/530)) ([8c16fa4](https://www.github.com/googleapis/java-bigtable/commit/8c16fa4c5290f67c43392953095bd759c2505bdb))


### Bug Fixes

* make keepalive params a bit conservative ([#528](https://www.github.com/googleapis/java-bigtable/issues/528)) ([f5f66c9](https://www.github.com/googleapis/java-bigtable/commit/f5f66c9ce019a6a24ddfe1efc0141835f5a02f34))
---


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
ad548 pushed a commit to ad548/java-bigtable that referenced this pull request Mar 13, 2021
🤖 I have created a release \*beep\* \*boop\* 
---
## [1.18.0](https://www.github.com/googleapis/java-bigtable/compare/v1.17.3...v1.18.0) (2020-11-13)


### Features

* **autogenerated:** Update BigtableTableAdmin GetIamPolicy to include the additional binding for Backup, Change DeleteAppProfileRequest.ignore_warnings to REQUIRED. ([googleapis#530](https://www.github.com/googleapis/java-bigtable/issues/530)) ([8c16fa4](https://www.github.com/googleapis/java-bigtable/commit/8c16fa4c5290f67c43392953095bd759c2505bdb))


### Bug Fixes

* make keepalive params a bit conservative ([googleapis#528](https://www.github.com/googleapis/java-bigtable/issues/528)) ([f5f66c9](https://www.github.com/googleapis/java-bigtable/commit/f5f66c9ce019a6a24ddfe1efc0141835f5a02f34))
---


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
Labels
api: bigtable Issues related to the googleapis/java-bigtable API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants