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

cdc:ignore timeout if snapshot is not complete #18959

Merged
merged 18 commits into from
Dec 13, 2022

Conversation

subodh1810
Copy link
Contributor

Ref : https://airbytehq-team.slack.com/archives/C046N0TGRMX/p1667552437922099?thread_ts=1667481029.484859&cid=C046N0TGRMX

This change allows to ignore the SUBSEQUENT_RECORD_WAIT_TIME timeout if a snapshot is going on.

@subodh1810 subodh1810 self-assigned this Nov 4, 2022
@subodh1810 subodh1810 requested a review from a team as a code owner November 4, 2022 11:34
@github-actions github-actions bot added the area/connectors Connector related issues label Nov 4, 2022
@subodh1810
Copy link
Contributor Author

subodh1810 commented Nov 4, 2022

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3393463224
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3393463224
No Python unittests run

Build Passed

Test summary info:

All Passed

@subodh1810
Copy link
Contributor Author

subodh1810 commented Nov 4, 2022

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3393463455
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3393463455
No Python unittests run

Build Passed

Test summary info:

All Passed

@subodh1810
Copy link
Contributor Author

subodh1810 commented Nov 4, 2022

/test connector=connectors/source-mssql

🕑 connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/3393463741
✅ connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/3393463741
No Python unittests run

Build Passed

Test summary info:

All Passed

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2022

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

  • Run integration tests
  • Bump connector or module version
  • Add changelog
  • Publish the new version

❌ Sources (8)

Connector Version Changelog Publish
source-alloydb 1.0.33
(changelog missing)
source-alloydb-strict-encrypt 1.0.33 🔵
(ignored)
🔵
(ignored)
source-mssql 0.4.26
source-mssql-strict-encrypt 0.4.26 🔵
(ignored)
🔵
(ignored)
source-mysql 1.0.16
source-mysql-strict-encrypt 1.0.16 🔵
(ignored)
🔵
(ignored)
source-postgres 1.0.33
source-postgres-strict-encrypt 1.0.33 🔵
(ignored)
🔵
(ignored)
  • See "Actionable Items" below for how to resolve warnings and errors.

✅ Destinations (0)

Connector Version Changelog Publish
  • See "Actionable Items" below for how to resolve warnings and errors.

✅ Other Modules (0)

Actionable Items

(click to expand)

Category Status Actionable Item
Version
mismatch
The version of the connector is different from its normal variant. Please bump the version of the connector.

doc not found
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug.
Changelog
doc not found
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug.

changelog missing
There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish
not in seed
The connector is not in the seed file (e.g. source_definitions.yaml), so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that it is not a bug.

diff seed version
The connector exists in the seed file, but the latest version is not listed there. This usually means that the latest version is not published. Please use the /publish command to publish the latest version.

@subodh1810
Copy link
Contributor Author

subodh1810 commented Nov 4, 2022

/test connector=connectors/source-mssql

🕑 connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/3396521927
✅ connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/3396521927
No Python unittests run

Build Passed

Test summary info:

All Passed

@subodh1810
Copy link
Contributor Author

subodh1810 commented Nov 4, 2022

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3396522556
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3396522556
No Python unittests run

Build Passed

Test summary info:

All Passed

@subodh1810
Copy link
Contributor Author

subodh1810 commented Nov 4, 2022

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3396524244
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3396524244
No Python unittests run

Build Passed

Test summary info:

All Passed

@subodh1810 subodh1810 temporarily deployed to more-secrets November 4, 2022 19:23 Inactive
LOGGER.info("Closing cause next is returned as null");
requestClose();
if ((!receivedFirstRecord || hasSnapshotFinished || maxInstanceOfNoRecordsFound >= 10) && !signalledClose) {
LOGGER.info("Closing cause next is returned as null");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment or add a better log message here to reflect the new logic?

@subodh1810
Copy link
Contributor Author

subodh1810 commented Nov 7, 2022

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3409203595
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3409203595
No Python unittests run

Build Passed

Test summary info:

All Passed

@subodh1810
Copy link
Contributor Author

subodh1810 commented Nov 7, 2022

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3409204619
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3409204619
No Python unittests run

Build Passed

Test summary info:

All Passed

@subodh1810
Copy link
Contributor Author

subodh1810 commented Nov 7, 2022

/test connector=connectors/source-mssql

🕑 connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/3409206076
❌ connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/3409206076
🐛 https://gradle.com/s/36yyuzzwuabxu

Build Failed

Test summary info:

Could not find result summary

@subodh1810 subodh1810 temporarily deployed to more-secrets November 7, 2022 09:44 Inactive
@subodh1810
Copy link
Contributor Author

subodh1810 commented Nov 7, 2022

/test connector=connectors/source-mssql

🕑 connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/3411529069
✅ connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/3411529069
No Python unittests run

Build Passed

Test summary info:

All Passed

@subodh1810 subodh1810 requested a review from akashkulk November 7, 2022 15:14
@@ -80,9 +82,12 @@ protected ChangeEvent<String, String> computeNext() {
// if within the timeout, the consumer could not get a record, it is time to tell the producer to
// shutdown.
if (next == null) {
LOGGER.info("Closing cause next is returned as null");
requestClose();
if ((!receivedFirstRecord || hasSnapshotFinished || maxInstanceOfNoRecordsFound >= 10) && !signalledClose) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In what case do we want to keep trying after queue gave us null?

Copy link
Contributor Author

@subodh1810 subodh1810 Nov 11, 2022

Choose a reason for hiding this comment

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

So the thing that happened with carecru's connection. Null was returned during snapshot cause Debezium took more time that 1 minute. We should have kept trying instead of shutting down

Copy link
Contributor

Choose a reason for hiding this comment

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

This should hopefully be addressed by heartbeats

@@ -46,6 +46,7 @@ public class DebeziumRecordIterator extends AbstractIterator<ChangeEvent<String,
private boolean receivedFirstRecord;
private boolean hasSnapshotFinished;
private boolean signalledClose;
private int maxInstanceOfNoRecordsFound;
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding heartbeats make it unnecessary but since only postgres has heartbeats for now I'll make sure it doesn't collide with heartbeats

@subodh1810
Copy link
Contributor Author

subodh1810 commented Dec 12, 2022

/publish connector=connectors/source-alloydb

🕑 Publishing the following connectors:
connectors/source-alloydb
https://github.com/airbytehq/airbyte/actions/runs/3679022860


Connector Did it publish? Were definitions generated?
connectors/source-alloydb

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@subodh1810
Copy link
Contributor Author

subodh1810 commented Dec 12, 2022

/publish connector=connectors/source-alloydb-strict-encrypt

🕑 Publishing the following connectors:
connectors/source-alloydb-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3679023383


Connector Did it publish? Were definitions generated?
connectors/source-alloydb-strict-encrypt

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@subodh1810
Copy link
Contributor Author

subodh1810 commented Dec 12, 2022

/publish connector=connectors/source-postgres

🕑 Publishing the following connectors:
connectors/source-postgres
https://github.com/airbytehq/airbyte/actions/runs/3679025005


Connector Did it publish? Were definitions generated?
connectors/source-postgres

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@subodh1810
Copy link
Contributor Author

subodh1810 commented Dec 12, 2022

/publish connector=connectors/source-postgres-strict-encrypt

🕑 Publishing the following connectors:
connectors/source-postgres-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3679025290


Connector Did it publish? Were definitions generated?
connectors/source-postgres-strict-encrypt

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@subodh1810
Copy link
Contributor Author

subodh1810 commented Dec 12, 2022

/publish connector=connectors/source-mysql-strict-encrypt

🕑 Publishing the following connectors:
connectors/source-mysql-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3679026952


Connector Did it publish? Were definitions generated?
connectors/source-mysql-strict-encrypt

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@subodh1810 subodh1810 temporarily deployed to more-secrets December 12, 2022 19:18 — with GitHub Actions Inactive
@subodh1810
Copy link
Contributor Author

subodh1810 commented Dec 12, 2022

/publish connector=connectors/source-mssql-strict-encrypt

🕑 Publishing the following connectors:
connectors/source-mssql-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3679027337


Connector Did it publish? Were definitions generated?
connectors/source-mssql-strict-encrypt

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@subodh1810
Copy link
Contributor Author

subodh1810 commented Dec 12, 2022

/publish connector=connectors/source-mysql

🕑 Publishing the following connectors:
connectors/source-mysql
https://github.com/airbytehq/airbyte/actions/runs/3679028228


Connector Did it publish? Were definitions generated?
connectors/source-mysql

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@subodh1810
Copy link
Contributor Author

subodh1810 commented Dec 12, 2022

/publish connector=connectors/source-mssql

🕑 Publishing the following connectors:
connectors/source-mssql
https://github.com/airbytehq/airbyte/actions/runs/3679029796


Connector Did it publish? Were definitions generated?
connectors/source-mssql

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@subodh1810 subodh1810 temporarily deployed to more-secrets December 12, 2022 19:18 — with GitHub Actions Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets December 12, 2022 19:30 — with GitHub Actions Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets December 12, 2022 19:31 — with GitHub Actions Inactive
@subodh1810
Copy link
Contributor Author

subodh1810 commented Dec 12, 2022

/publish connector=connectors/source-mssql-strict-encrypt

🕑 Publishing the following connectors:
connectors/source-mssql-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3679368169


Connector Did it publish? Were definitions generated?
connectors/source-mssql-strict-encrypt

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@subodh1810 subodh1810 temporarily deployed to more-secrets December 12, 2022 20:09 — with GitHub Actions Inactive
@subodh1810 subodh1810 temporarily deployed to more-secrets December 12, 2022 20:10 — with GitHub Actions Inactive
@subodh1810 subodh1810 temporarily deployed to more-secrets December 13, 2022 09:53 — with GitHub Actions Inactive
@subodh1810 subodh1810 temporarily deployed to more-secrets December 13, 2022 09:53 — with GitHub Actions Inactive
@subodh1810 subodh1810 temporarily deployed to more-secrets December 13, 2022 11:25 — with GitHub Actions Inactive
@subodh1810 subodh1810 temporarily deployed to more-secrets December 13, 2022 11:25 — with GitHub Actions Inactive
@subodh1810 subodh1810 merged commit 2ea6110 into master Dec 13, 2022
@subodh1810 subodh1810 deleted the snapshot-completion-waiting branch December 13, 2022 12:05
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.

5 participants