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 as a code owner Nov 11, 2020
@google-cla google-cla bot added the cla: yes label Nov 11, 2020
@sushanb
Copy link
Contributor Author

@sushanb sushanb commented Nov 11, 2020

@product-auto-label product-auto-label bot added the api: bigtable label Nov 11, 2020
@codecov
Copy link

@codecov 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 approved these changes Nov 13, 2020
@kolea2 kolea2 merged commit f5f66c9 into googleapis:master Nov 13, 2020
18 of 19 checks passed
@sushanb sushanb deleted the keepalive-conservatibe branch Nov 13, 2020
gcf-merge-on-green bot pushed a commit that referenced this issue 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 issue Mar 13, 2021
ad548 pushed a commit to ad548/java-bigtable that referenced this issue 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 cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants