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

Update Yugabyte wait strategy #7599

Closed
wants to merge 17 commits into from
Closed

Conversation

eddumelendez
Copy link
Member

Update wait strategy by using a temporal table instead of select statement.

Also, make it compatible with version 2.18.

HarshDaryani896 and others added 16 commits July 27, 2023 14:09
* Changed wait strategy for ysql testcontainer

* Updating YugabyteDB-YSQL WaitStrategy

* Catch IllegalStateException

* Update test query

* Changes as per review

* Added a test case.

* Updating YSQL wait strategy

* Removing redundant if.

* Minor Change

* YugabyteDB version updates in tests
* Changed wait strategy for ysql testcontainer

* Updating YugabyteDB-YSQL WaitStrategy

* Catch IllegalStateException

* Update test query

* Changes as per review

* Added a test case.

* Updating YSQL wait strategy

* Removing redundant if.

* Minor Change

* YugabyteDB version updates in tests
@eddumelendez eddumelendez removed this from the next milestone Oct 2, 2023
@eddumelendez
Copy link
Member Author

eddumelendez commented Oct 9, 2023

@HarshDaryani896 can you please take a look why test are failing? I think we can consider this one too. Thanks

@HarshDaryani896
Copy link
Contributor

HarshDaryani896 commented Oct 25, 2023

@eddumelendez We are working on figuring out the reason of the failure. Meanwhile I figured that if we create a normal table instead of a temp table as part of the YSQL waitStrategy we will not get this failure.

I have attached a patch to create and drop a table, instead of creating a temp table in the YSQL waitStrategy. This should resolve the failure.
RemoveTempTable.patch

@HarshDaryani896
Copy link
Contributor

Hey @eddumelendez I have created a new branch which has all the required changes including the above patch. Please let me know if you want me to create a new PR or if we can update the current PR. Thanks

@eddumelendez
Copy link
Member Author

@HarshDaryani896 feel free to raise a PR. I'm planning to include it for the next release. Thanks in advance

@HarshDaryani896
Copy link
Contributor

Hey @eddumelendez Thank you for the response.
I have created a new PR #7784 for this which has the above patch applied. We can close this one and go ahead and merge that PR.
Would it be possible for you to share ETA of the next release? Thanks.

@eddumelendez
Copy link
Member Author

Superseded by #7784

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants