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 & test qrep pause/unpause #1388

Merged
merged 18 commits into from
Mar 5, 2024
Merged

Fix & test qrep pause/unpause #1388

merged 18 commits into from
Mar 5, 2024

Conversation

serprex
Copy link
Contributor

@serprex serprex commented Feb 27, 2024

As is apt to happen when adding tests, also fix pause/unpause for qrep/xmin

  1. waitForNewRows would prevent pausing; use a selector to wait on signals while waiting on new rows while waiting on context cancelation
  2. CurrentFlowStatus wasn't being set to RUNNING when exiting from PAUSED

As an aside, it turns out all the qrep/xmin tests are doing InitialCopyOnly besides this new test

Also remove DisableWaitForNewRows because it's unused & is meant to diverge testing from reality

@serprex serprex force-pushed the test-pause-qrep-xmin branch 3 times, most recently from a5421c5 to 6d714f2 Compare February 27, 2024 18:45
@serprex
Copy link
Contributor Author

serprex commented Feb 27, 2024

Failures here are showing that temporal testsuite is not suitable for integration testing because ContinueAsNew isn't handled

temporalio/sdk-go#805

Looks like even children workflows return continue as new error instead of behaving as they should

@serprex serprex marked this pull request as draft February 27, 2024 20:54
@serprex serprex force-pushed the test-pause-qrep-xmin branch from 6d714f2 to a19df84 Compare March 4, 2024 14:59
@serprex serprex marked this pull request as ready for review March 4, 2024 15:00
@iskakaushik
Copy link
Contributor

@serprex is there a way to prevent INFO logs from showing up in tests?

***"time":"2024-03-04T15:06:03.6158228Z","level":"INFO","msg":"Sent Standby status message. processed 0 rows","Namespace":"default","TaskQueue":"peer-flow-task-queue","WorkerID":"4459@vm8n9339@","ActivityID":"11","ActivityType":"MaintainPull","Attempt":1,"WorkflowType":"SyncFlowWorkflow","WorkflowID":"sync-flow-test_simple_schema_changes_bq_idwud1dh_20240304150318-f97680e0-504d-438e-8b02-372c948200e3","RunID":"9b958103-daaa-48ad-aa90-0f4093f79338","deploymentUid":""***

@serprex
Copy link
Contributor Author

serprex commented Mar 4, 2024

Yes, we'd have to pass a log level when invoking peer-flow

Will do so in a separate PR

(I tried to avoid some issue by using go test without -v so that it'd buffer logs & spit them out in chunks. The ideal would be for peer-flow to run in another step in background, but alas, github actions don't handle stdout continuing to flow to a finished step)

@serprex serprex force-pushed the test-pause-qrep-xmin branch from c68cd13 to b46ff1e Compare March 4, 2024 15:41
@serprex serprex force-pushed the test-pause-qrep-xmin branch from 779263c to c64baba Compare March 4, 2024 19:28
@serprex serprex force-pushed the test-pause-qrep-xmin branch from bc0a7cc to 9d5bff7 Compare March 5, 2024 00:46
@serprex serprex enabled auto-merge (squash) March 5, 2024 04:38
@serprex serprex changed the title Test qrep pause/unpause Fix & test qrep pause/unpause Mar 5, 2024
@serprex serprex merged commit 3e3e2ab into main Mar 5, 2024
7 checks passed
@serprex serprex deleted the test-pause-qrep-xmin branch March 5, 2024 15:05
heavycrystal added a commit that referenced this pull request Mar 5, 2024
`postgresql` appears to be a plugin that doesn't support Temporal
[advanced
visibility](https://docs.temporal.io/visibility#advanced-visibility)
which we use in `flow-api`. It now crashes on startup.

Reverting the change made in #1388
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants