-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Fixing Debezium integration tests #11154
Conversation
0664a34
to
1cf310b
Compare
1cf310b
to
7df762f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7df762f
to
2b16e1c
Compare
/pulsarbot run-failure-checks |
Pulsar IO integration test failed after rebase, likely the same reason as #11204 (comment) |
I confirmed that tests pass after revert of PIP-85; @nlu90 is looking at the options to fix this without revert. |
2b16e1c
to
5de205b
Compare
Failed test |
5de205b
to
e60b186
Compare
@nlu90 @eolivelli situation with this PR so far:
I reverted both PRs #11251 and #11056 locally; rebuilt/re-run the test and the test passed, "Timed out waiting to flush" is not logged in the function log |
tests do pass if I create a new pulsar client in the PulsarOffsetBackingStore's ctor instead of the one passed there (I hardcoded the url in the test). There is still something fishy with passing the client via source context and, ironically, I am missing on a context of what's going on there. Feels like some kind of deadlock at the PulsarOffsetStore.set() where it does I suggest we merge this PR, and @nlu90 continues work on the client in the SourceContext with the test enabled on the CI. |
@dlg99 Let me know when you merge this PR, I will continue look into the fix. |
@nlu90 I don't have permissions to merge; I hope @codelipenghui / @sijie / @merlimat / @eolivelli or other committer can merge. |
@nlu90 are you "approving" this patch ? |
I am fine to merge this patch as soon as CI is green |
I don't know what's going on with "CI - Unit - Brokers - Broker Group 1 / unit-tests" but it is not related. |
e60b186
to
0b24be0
Compare
I think the agreement is to let Neng work on a proper fix based on Andrey's PR. |
I pushed a fix for the debezium connector in #11435 And the integration test passed on my local laptop:
|
@nlu90 I rebased the PR on top of recent master, the IO tests still failed.
|
/pulsarbot run-failure-checks |
- check for flush lsn updates
0b24be0
to
e0f6a4e
Compare
/pulsarbot run-failure-checks |
Excluded tests for sinks; something times out there, "Pulsar-IO Sinks and Sources" is limited to sources now (but passes) |
/pulsarbot run-failure-checks |
Fixes apache#11099 ### Motivation Integration test for debezium connector to tun on CI ### Modifications Debezium integration test for postgres: added check for flush lsn updates Added pulsar-io tests to the CI
Fixes #8502 ### Motivation Upgrade Debezium to a newer version ### Modifications Upgraded Deebzium to v.1.5.4 (latest built with Java 8, v.1.6.x built with Java 11) Upgraded kafka-client to 2.7 (version debezium tested with) Scala-lib to 2.13.6 (for kafka-client) Dealt with API changes, tests etc. PR is on top of #11154 to have Debezium integration tests on CI.
Fixes apache#11099 ### Motivation Integration test for debezium connector to tun on CI ### Modifications Debezium integration test for postgres: added check for flush lsn updates Added pulsar-io tests to the CI
Fixes apache#8502 ### Motivation Upgrade Debezium to a newer version ### Modifications Upgraded Deebzium to v.1.5.4 (latest built with Java 8, v.1.6.x built with Java 11) Upgraded kafka-client to 2.7 (version debezium tested with) Scala-lib to 2.13.6 (for kafka-client) Dealt with API changes, tests etc. PR is on top of apache#11154 to have Debezium integration tests on CI.
Fixes #11099
Motivation
Integration test for debezium connector to tun on CI
Modifications
Debezium integration test for postgres: added check for flush lsn updates
Added pulsar-io tests to the CI
Verifying this change
Ran locally:
This change fixes existing tests.
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesNO
Documentation