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.

Sign up for GitHub

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

Jump to bottom

feat: long running transaction clean up background task. Adding configuration options for closing inactive transactions. #2419

Merged
merged 57 commits into from Aug 1, 2023
Merged

feat: long running transaction clean up background task. Adding configuration options for closing inactive transactions. #2419

merged 57 commits into from Aug 1, 2023

Conversation

arpan14
Copy link
Collaborator

@arpan14 arpan14 commented May 9, 2023 •
edited

Adding a couple of option in SessionPoolOptions for auto-detecting long running transactions, logging the stack-trace and removing such transactions from consuming more resources. There can be scenarios where transactions are unexpectedly taking more time and hence can consume the session for longer durations. With this change, customers get two new options which can be set while providing SessionPoolOptions

  1. setWarnIfInactiveTransactions() - Calling this method will generate warning logs and help in identifying faulty code which is leading to incorrect consumption of sessions.
  2. setWarnAndCloseIfInactiveTransactions() - Calling this method will close the faulty transactions that are consuming the resources. We will also be generating the warning logs which can be later referred to know the transactions that were closed through this option.

Note : The feature is currently under development. The final usage pattern will be updated in a subsequent commit.

arpan14 and others added 7 commits February 8, 2023 12:22
@arpan14
fix: prevent illegal negative timeout values into thread sleep() meth…
edc5bbf 
…od while retrying exceptions in unit tests.

* For details on issue see - googleapis#2206
@arpan14
Merge pull request #1 from arpan14/retryerror
49a85df 
fix: prevent illegal negative timeout values into thread sleep() method while retrying exceptions in unit tests.
@arpan14
Fixing lint issues.
4cd497b
@arpan14
Merge branch 'googleapis:main' into main
4a6aa8e
@arpan14
Merge branch 'googleapis:main' into main
b2aa09d
@arpan14
Merge branch 'googleapis:main' into main
8d6d71e
@arpan14
feat: long running transaction clean up background task. Adding confi…
34502d1 
…guration options for closing inactive transactions.
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels May 9, 2023
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels May 9, 2023
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: xl Pull request size is extra large. labels May 9, 2023
@arpan14 arpan14 marked this pull request as ready for review May 10, 2023 10:29
@arpan14 arpan14 requested a review from a team as a code owner May 10, 2023 10:29
@arpan14 arpan14 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 10, 2023
@arpan14 arpan14 requested a review from asthamohta May 11, 2023 06:54
olavloite
olavloite reviewed May 22, 2023
View reviewed changes
Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log of the integration tests for the emulator (and possibly also for the other test runs) are spammed with a very large number of warnings for closed long-running transactions.

  1. Is that expected? Or are we accidentally removing a lot more sessions from the pool than we should?
  2. If it is expected, can we somehow silence that log? It now makes the log too large to be really useful.

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java Show resolved Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java Outdated Show resolved Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java Show resolved Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java Outdated Show resolved Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java Outdated Show resolved Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java Outdated Show resolved Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java Outdated Show resolved Hide resolved
google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java Outdated Show resolved Hide resolved
google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java Outdated Show resolved Hide resolved
google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java Show resolved Hide resolved
arpan14 and others added 4 commits June 6, 2023 17:18
@arpan14 @olavloite
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
7858d7c 
…ssionPool.java

Co-authored-by: Knut Olav Løite <koloite@gmail.com>
@arpan14 @olavloite
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
342eed8 
…ssionPoolOptions.java

Co-authored-by: Knut Olav Løite <koloite@gmail.com>
@arpan14 @olavloite
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
227dd27 
…ssionPoolOptions.java

Co-authored-by: Knut Olav Løite <koloite@gmail.com>
@arpan14 @olavloite
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
ab05e08 
…ssionPoolOptions.java

Co-authored-by: Knut Olav Løite <koloite@gmail.com>
@arpan14 arpan14 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 11, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 11, 2023
rajatbhatta
rajatbhatta approved these changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@rajatbhatta rajatbhatta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@arpan14 arpan14 added the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 25, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 25, 2023
@gcf-owl-bot gcf-owl-bot bot requested a review from a team as a code owner July 25, 2023 07:41
@arpan14 arpan14 added the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 1, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 1, 2023
@arpan14 arpan14 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 1, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 1, 2023
@arpan14 arpan14 merged commit 423e1a4 into googleapis:main Aug 1, 2023
21 of 22 checks passed
@arpan14 arpan14 deleted the session-leak-pr branch August 1, 2023 13:54
@release-please release-please bot mentioned this pull request Aug 1, 2023
Merged
@arpan14 arpan14 mentioned this pull request Aug 2, 2023
Merged
gcf-merge-on-green bot pushed a commit that referenced this pull request Aug 5, 2023
@release-please
chore(main): release 6.45.0 (#2560)
08af671 
🤖 I have created a release *beep* *boop*
---


## [6.45.0](https://togithub.com/googleapis/java-spanner/compare/v6.44.0...v6.45.0) (2023-08-04)


### Features

* Enable leader aware routing by default in Connection API. This enables its use in the JDBC driver and PGAdapter. The update contains performance optimisations that will reduce the latency of read/write transactions that originate from a region other than the default leader region. ([2a85446](https://togithub.com/googleapis/java-spanner/commit/2a85446b162b006ce84a86285af1767c879b27ed))
* Enable leader aware routing by default. This update contains performance optimisations that will reduce the latency of read/write transactions that originate from a region other than the default leader region. ([441c1b0](https://togithub.com/googleapis/java-spanner/commit/441c1b03c3e976c6304a99fefd93b5c4291e5364))
* Long running transaction clean up background task. Adding configuration options for closing inactive transactions. ([#2419](https://togithub.com/googleapis/java-spanner/issues/2419)) ([423e1a4](https://togithub.com/googleapis/java-spanner/commit/423e1a4b483798d9683ff9bd232b53d76e09beb0))
* Support partitioned queries + data boost in Connection API ([#2540](https://togithub.com/googleapis/java-spanner/issues/2540)) ([4e31d04](https://togithub.com/googleapis/java-spanner/commit/4e31d046f5d80abe8876a729ddba045c70f3261d))


### Bug Fixes

* Apply stream wait timeout ([#2544](https://togithub.com/googleapis/java-spanner/issues/2544)) ([5a12cd2](https://togithub.com/googleapis/java-spanner/commit/5a12cd29601253423c5738be5471a036fd0334be))


### Dependencies

* Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.14.0 ([#2562](https://togithub.com/googleapis/java-spanner/issues/2562)) ([dbd5c75](https://togithub.com/googleapis/java-spanner/commit/dbd5c75be39262003092ff4a925ed470cc45f8be))
* Update dependency org.openjdk.jmh:jmh-core to v1.37 ([#2565](https://togithub.com/googleapis/java-spanner/issues/2565)) ([d5c36bf](https://togithub.com/googleapis/java-spanner/commit/d5c36bfbb67ecb14854944779da6e4dbd93f3559))
* Update dependency org.openjdk.jmh:jmh-generator-annprocess to v1.37 ([#2566](https://togithub.com/googleapis/java-spanner/issues/2566)) ([73e92d4](https://togithub.com/googleapis/java-spanner/commit/73e92d42fe6d334b6efa6485246dc67858adb0a9))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
psinghbay1
psinghbay1 reviewed Sep 13, 2023
View reviewed changes
google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java
*
* @return this builder for chaining
*/
Builder setWarnAndCloseIfInactiveTransactions() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we making these methods package protected?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@psinghbay1 This feature is not yet launched. Hence it is package protected in this PR. We will be enabling this in the next couple of weeks.

@psinghbay1
Copy link

Any reason we are keeping these methods package protected in builder class?

@arpan14 arpan14 mentioned this pull request Oct 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@psinghbay1 psinghbay1 psinghbay1 left review comments

@olavloite olavloite olavloite approved these changes

@asthamohta asthamohta asthamohta approved these changes

@rajatbhatta rajatbhatta rajatbhatta approved these changes

@harshachinta harshachinta Awaiting requested review from harshachinta

Assignees
No one assigned
Labels
api: spanner Issues related to the googleapis/java-spanner API. size: xl Pull request size is extra large.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@arpan14 @psinghbay1 @olavloite @asthamohta @rajatbhatta @yoshi-kokoro