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

Replace ReadOnlyTransaction() with Single() #984

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

manitgupta
Copy link
Member

During mocking the Single() method of the client library, we used the ReadOnlyTransaction() method to return an instance of a ReadOnlyTransaction. This is different from the ReadOnlyTransaction object that gets returned from the Single() method, which is more suited to single use transactions that we use in the expressions API for verification.

This caused the nature of the ReadOnlyTransactions being used to change from being SingleUse to non-SingleUse, which would then cause session leaks since they were not automatically getting cleaned up.

Using Single() fixes the session leak, and has been validated by running 20K CHECK constraint validations.

@manitgupta manitgupta requested a review from asthamohta January 7, 2025 10:08
@manitgupta manitgupta requested a review from a team as a code owner January 7, 2025 10:08
@manitgupta manitgupta requested review from darshan-sj and removed request for a team January 7, 2025 10:08
@manitgupta manitgupta merged commit 1a751d7 into GoogleCloudPlatform:master Jan 7, 2025
4 checks passed
akashthawaitcc added a commit to ollionorg/spanner-migration-tool-fork that referenced this pull request Jan 7, 2025
akashthawaitcc added a commit to ollionorg/spanner-migration-tool-fork that referenced this pull request Jan 7, 2025
* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (GoogleCloudPlatform#978) (#30)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------



* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------



* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------



* fixed if condition

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984) (#35)

Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>
asthamohta pushed a commit that referenced this pull request Jan 7, 2025
* UI changes for check constraint support

* removed url changed for testing

* Check UI cmt (#29)

* remove the block which will not execute

* refactor the code
1. remove the unreachable code
2.  remvoe the node-sql-parser package

* refactor the code
1. rename the generateId method

---------

Co-authored-by: Vivek Yadav <[email protected]>

* updated the dist file and package.lock file

* Develop (#36)

* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (#978) (#30)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------



* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------



* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------



* fixed if condition

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (#984) (#35)

Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: taherkl <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>
akashthawaitcc added a commit to ollionorg/spanner-migration-tool-fork that referenced this pull request Jan 8, 2025
* feat: default values final PR (GoogleCloudPlatform#976)

* changes

* changes

* changes

* changes

* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (GoogleCloudPlatform#978)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------

Co-authored-by: Vivek Yadav <[email protected]>

* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------

Co-authored-by: Vivek Yadav <[email protected]>

* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------

Co-authored-by: Vivek Yadav <[email protected]>

* fixed if condition

---------

Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984)

* UI changes for check constraint support (GoogleCloudPlatform#979)

* UI changes for check constraint support

* removed url changed for testing

* Check UI cmt (#29)

* remove the block which will not execute

* refactor the code
1. remove the unreachable code
2.  remvoe the node-sql-parser package

* refactor the code
1. rename the generateId method

---------

Co-authored-by: Vivek Yadav <[email protected]>

* updated the dist file and package.lock file

* Develop (#36)

* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (GoogleCloudPlatform#978) (#30)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------



* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------



* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------



* fixed if condition

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984) (#35)

Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: taherkl <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Astha Mohta <[email protected]>
Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>
akashthawaitcc added a commit to ollionorg/spanner-migration-tool-fork that referenced this pull request Jan 8, 2025
* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984)

* UI changes for check constraint support (GoogleCloudPlatform#979)

* UI changes for check constraint support

* removed url changed for testing

* Check UI cmt (#29)

* remove the block which will not execute

* refactor the code
1. remove the unreachable code
2.  remvoe the node-sql-parser package

* refactor the code
1. rename the generateId method

---------

Co-authored-by: Vivek Yadav <[email protected]>

* updated the dist file and package.lock file

* Develop (#36)

* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (GoogleCloudPlatform#978) (#30)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------



* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------



* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------



* fixed if condition

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984) (#35)

Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: taherkl <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Manit Gupta <[email protected]>
Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
akashthawaitcc added a commit to ollionorg/spanner-migration-tool-fork that referenced this pull request Jan 8, 2025
* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (GoogleCloudPlatform#978) (#30)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------



* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------



* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------



* fixed if condition

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984) (#35)

Co-authored-by: Manit Gupta <[email protected]>

* sync upstream/master (#42)

* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984)

* UI changes for check constraint support (GoogleCloudPlatform#979)

* UI changes for check constraint support

* removed url changed for testing

* Check UI cmt (#29)

* remove the block which will not execute

* refactor the code
1. remove the unreachable code
2.  remvoe the node-sql-parser package

* refactor the code
1. rename the generateId method

---------

Co-authored-by: Vivek Yadav <[email protected]>

* updated the dist file and package.lock file

* Develop (#36)

* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (GoogleCloudPlatform#978) (#30)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------



* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------



* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------



* fixed if condition

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984) (#35)

Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: taherkl <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Manit Gupta <[email protected]>
Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>
bharadwaj-aditya pushed a commit that referenced this pull request Jan 17, 2025
* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (#978) (#30)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------



* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------



* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------



* fixed if condition

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (#984) (#35)

Co-authored-by: Manit Gupta <[email protected]>

* sync upstream/master (#42)

* Replace ReadOnlyTransaction() with Single() (#984)

* UI changes for check constraint support (#979)

* UI changes for check constraint support

* removed url changed for testing

* Check UI cmt (#29)

* remove the block which will not execute

* refactor the code
1. remove the unreachable code
2.  remvoe the node-sql-parser package

* refactor the code
1. rename the generateId method

---------

Co-authored-by: Vivek Yadav <[email protected]>

* updated the dist file and package.lock file

* Develop (#36)

* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (#978) (#30)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------



* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------



* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------



* fixed if condition

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (#984) (#35)

Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: taherkl <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Manit Gupta <[email protected]>
Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* added updated build file

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>
akashthawaitcc added a commit to ollionorg/spanner-migration-tool-fork that referenced this pull request Jan 23, 2025
* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (GoogleCloudPlatform#978) (#30)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------



* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------



* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------



* fixed if condition

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984) (#35)

Co-authored-by: Manit Gupta <[email protected]>

* sync upstream/master (#42)

* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984)

* UI changes for check constraint support (GoogleCloudPlatform#979)

* UI changes for check constraint support

* removed url changed for testing

* Check UI cmt (#29)

* remove the block which will not execute

* refactor the code
1. remove the unreachable code
2.  remvoe the node-sql-parser package

* refactor the code
1. rename the generateId method

---------

Co-authored-by: Vivek Yadav <[email protected]>

* updated the dist file and package.lock file

* Develop (#36)

* verification API and dump flow changes to support MySQL CHECK CONSTRAINTS (GoogleCloudPlatform#978) (#30)

* verification ap and dump flow changes

* fixed IT issue

* Check constraints verificartion api v2 (#24)

* handled function not found

* added unhandled error

* updated the error msg

---------



* fix IT issue

* comment addressed (#27)

* comment addressed
1. rename the functionNotFound
2. added condition to call verification api

* spell checked

---------



* refactor the DbDumpImpl struct (#28)

* refactor the DbDumpImpl struct

* remove the GenerateCheckConstrainstExprId method

---------



* fixed if condition

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* Replace ReadOnlyTransaction() with Single() (GoogleCloudPlatform#984) (#35)

Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: taherkl <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>

---------

Co-authored-by: Manit Gupta <[email protected]>
Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>

* updated the build and resolve the issue
1. verify api issue
2. drop column issue
3. updated the build

* updated the build

---------

Co-authored-by: Taher Lakdawala <[email protected]>
Co-authored-by: Akash Thawait <[email protected]>
Co-authored-by: taherkl <[email protected]>
Co-authored-by: Vivek Yadav <[email protected]>
Co-authored-by: Manit Gupta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants