[Source-MsSQL] Set default cursor for cdc mode #29493

nguyenaiden · 2023-08-16T18:17:15Z

What

To prepare for Destination v2, Cdc-enabled DB sources need to have a pre-defined cursor for normalization to operate properly.

How

During the discover phase, source_defined_cursor is set to true, and a new composite cursor column called _ab_cdc_cursor is selected as the predefined cursor. This column is constructed from:

Converting the emittedAt timestamp , generated at the read step, to epoch seconds.
Convert that to nanoseconds.
Initialize a threadsafe recordCounter = 1
Add the recordCounter to the converted timestamp
Increment the counter

The numeric cursor value will be computed for every record and allows for fast comparison in tie-breaking scenarios for records with the same Primary Key.

Updated Streamstate
{
  "cdc": false,
  "streams": [
    {
      "stream_name": "users",
      "cursor_field": [
        "_ab_cdc_cursor"
      ],
      "stream_namespace": "dbo"
    }
  ],

🚨 User Impact 🚨

New CDC syncs that have refreshed their source schema will see this cursor field be chosen.
This update requires that users do a full reset OR drop their SCD tables to ensure that syncs continue operating normally
Customers who reset their data will continue to see old behavior until their source goes through the discover() phase again.

github-actions · 2023-08-16T18:17:41Z

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

PR name follows PR naming conventions
Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
Secrets in the connector's spec are annotated with airbyte_secret
All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

Check for hidden checklists in your PR description
Toggle the github label checklist-action-run on/off to re-run the checklist CI.

octavia-squidington-iii · 2023-08-16T18:39:20Z

source-mssql test report (commit `c6f9dee920`) - ❌

⏲️ Total pipeline duration: 20mn03s

Step	Result
Validate airbyte-integrations/connectors/source-mssql/metadata.yaml	✅
Connector version semver check	✅
Connector version increment check	❌
QA checks	✅
Build connector tar	✅
Build source-mssql docker image for platform linux/x86_64	✅
./gradlew :airbyte-integrations:connectors:source-mssql:integrationTest	❌
Acceptance tests	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql test

octavia-squidington-iii · 2023-08-16T18:59:50Z

source-mssql test report (commit `c6f9dee920`) - ❌

⏲️ Total pipeline duration: 18mn48s

Step	Result
Validate airbyte-integrations/connectors/source-mssql/metadata.yaml	✅
Connector version semver check	✅
Connector version increment check	❌
QA checks	✅
Build connector tar	✅
Build source-mssql docker image for platform linux/x86_64	✅
./gradlew :airbyte-integrations:connectors:source-mssql:integrationTest	❌
Acceptance tests	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql test

octavia-squidington-iii · 2023-08-16T19:30:40Z

source-mssql test report (commit `b056c83f60`) - ❌

⏲️ Total pipeline duration: 20mn24s

Step	Result
Validate airbyte-integrations/connectors/source-mssql/metadata.yaml	✅
Connector version semver check	✅
Connector version increment check	❌
QA checks	✅
Build connector tar	✅
Build source-mssql docker image for platform linux/x86_64	✅
./gradlew :airbyte-integrations:connectors:source-mssql:integrationTest	❌
Acceptance tests	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql test

nguyenaiden · 2023-08-16T20:41:58Z

/legacy-test connector=connectors/source-mssql

🕑 connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/5883654001
❌ connectors/source-mssql https://github.com/airbytehq/airbyte/actions/runs/5883654001
🐛 https://gradle.com/s/3abp55vbaktby

Build Failed

Test summary info:

Could not find result summary

octavia-squidington-iii · 2023-08-16T21:04:29Z

source-mssql test report (commit `67e3ec3cc1`) - ❌

⏲️ Total pipeline duration: 20mn51s

Step	Result
Validate airbyte-integrations/connectors/source-mssql/metadata.yaml	✅
Connector version semver check	✅
Connector version increment check	❌
QA checks	✅
Build connector tar	✅
Build source-mssql docker image for platform linux/x86_64	✅
./gradlew :airbyte-integrations:connectors:source-mssql:integrationTest	❌
Acceptance tests	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql test

octavia-squidington-iii · 2023-08-16T21:27:22Z

source-mssql test report (commit `bc733328b1`) - ❌

⏲️ Total pipeline duration: 20mn27s

Step	Result
Validate airbyte-integrations/connectors/source-mssql/metadata.yaml	❌
Connector version semver check	✅
Connector version increment check	✅
QA checks	✅
Build connector tar	✅
Build source-mssql docker image for platform linux/x86_64	✅
./gradlew :airbyte-integrations:connectors:source-mssql:integrationTest	❌
Acceptance tests	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql test

prateekmukhedkar

I believe we need to do the changes for source-mssql-strict-encrypt also.

akashkulk · 2023-08-22T19:46:45Z

airbyte-integrations/connectors/source-mssql/Dockerfile

@@ -24,5 +24,5 @@ ENV APPLICATION source-mssql

 COPY --from=build /airbyte /airbyte

-LABEL io.airbyte.version=1.1.1
+LABEL io.airbyte.version=2.0.0


Do you also want to do this for mssql-strict-encrypt?

Good call. Will add that

akashkulk · 2023-08-22T19:48:33Z

.../connectors/source-mssql/src/main/java/io/airbyte/integrations/source/mssql/MssqlSource.java

@@ -453,10 +456,12 @@ public List<AutoCloseableIterator<AirbyteMessage>> getIncrementalIterators(
              MssqlCdcTargetPosition.getTargetPosition(database, sourceConfig.get(JdbcUtils.DATABASE_KEY).asText()), true, firstRecordWaitTime,
              OptionalInt.empty());

+      final MssqlCdcConnectorMetadataInjector mssqlCdcConnectorMetadataInjector = MssqlCdcConnectorMetadataInjector.getInstance(emittedAt);


One quick thought I had was :
if this is now stateful, do we want to declare this in the constructor to prevent other parts of the code from declaring multiple versions of the CdcConnectorMetadataInjector?

@akashkulk I actually already turned it into a singleton. Refer to MssqlCdcConnectorMetadataInjector.java

akashkulk

Approving to unblock. Some quick nits

akashkulk

Actually probably want to look into test failures before merging

nguyenaiden · 2023-08-22T20:53:03Z

/legacy-test connector=connectors/source-mssql

github-actions · 2023-08-22T21:30:56Z

source-mssql test report (commit `06d2655046`) - ❌

⏲️ Total pipeline duration: 38mn20s

Step	Result
Java Connector Unit Tests	✅
Build connector tar	✅
Build source-mssql docker image for platform linux/x86_64	✅
Java Connector Integration Tests	❌
Acceptance tests	✅
Validate airbyte-integrations/connectors/source-mssql/metadata.yaml	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	❌

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql test

github-actions · 2023-08-23T00:55:25Z

source-mssql test report (commit `f964644809`) - ❌

⏲️ Total pipeline duration: 18mn26s

Step	Result
Java Connector Unit Tests	✅
Build connector tar	✅
Build source-mssql docker image for platform linux/x86_64	✅
Java Connector Integration Tests	❌
Acceptance tests	✅
Validate airbyte-integrations/connectors/source-mssql/metadata.yaml	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	❌

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql test

Added migration guide Added breaking changes key to metadata

nguyenaiden · 2023-08-23T18:14:57Z

Successful test runs:
source-mssql: https://github.com/airbytehq/airbyte/actions/runs/5954575782/job/16151423518
source-mssql-strict-encrypt: https://github.com/airbytehq/airbyte/actions/runs/5954220427/job/16150814124

nguyenaiden · 2023-08-23T19:01:13Z

/approve-and-merge reason="legacy tests + all QA checks passed"

octavia-approvington · 2023-08-23T19:01:46Z

After a careful ML study,
we think this looks okay.

github-actions · 2023-08-23T22:04:03Z

source-mssql-strict-encrypt test report (commit `c23dcb3513`) - ❌

⏲️ Total pipeline duration: 316mn20s

Step	Result
Java Connector Unit Tests	❌
Build connector tar	✅
Build source-mssql-strict-encrypt docker image for platform linux/x86_64	✅
Java Connector Integration Tests	❌
Acceptance tests	❌
Validate airbyte-integrations/connectors/source-mssql-strict-encrypt/metadata.yaml	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	❌

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql-strict-encrypt test

github-actions · 2023-08-23T22:39:19Z

source-mssql test report (commit `c23dcb3513`) - ❌

⏲️ Total pipeline duration: 35mn06s

Step	Result
Java Connector Unit Tests	✅
Build connector tar	✅
Build source-mssql docker image for platform linux/x86_64	✅
Java Connector Integration Tests	❌
Acceptance tests	✅
Validate airbyte-integrations/connectors/source-mssql/metadata.yaml	✅
Connector version semver check	✅
Connector version increment check	❌
QA checks	❌

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql test

github-actions · 2023-08-23T22:48:09Z

source-mssql test report (commit `46a8fd444e`) - ❌

⏲️ Total pipeline duration: 06mn30s

Step	Result
Java Connector Unit Tests	✅
Build connector tar	✅
Build source-mssql docker image for platform linux/x86_64	✅
Java Connector Integration Tests	❌
Acceptance tests	✅
Validate airbyte-integrations/connectors/source-mssql/metadata.yaml	✅
Connector version semver check	✅
Connector version increment check	❌
QA checks	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql test

github-actions · 2023-08-23T22:52:19Z

source-mssql-strict-encrypt test report (commit `46a8fd444e`) - ❌

⏲️ Total pipeline duration: 04mn00s

Step	Result
Java Connector Unit Tests	❌
Build connector tar	✅
Build source-mssql-strict-encrypt docker image for platform linux/x86_64	✅
Java Connector Integration Tests	❌
Acceptance tests	❌
Validate airbyte-integrations/connectors/source-mssql-strict-encrypt/metadata.yaml	✅
Connector version semver check	✅
Connector version increment check	❌
QA checks	❌

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-mssql-strict-encrypt test

octavia-squidington-iii added area/connectors Connector related issues connectors/source/mssql labels Aug 16, 2023

nguyenaiden marked this pull request as ready for review August 16, 2023 18:18

nguyenaiden requested a review from a team as a code owner August 16, 2023 18:18

nguyenaiden requested review from rodireich, edgao, subodh1810, akashkulk and prateekmukhedkar August 16, 2023 20:43

nguyenaiden changed the title ~~[Source-MsSQL] Set default cursor for cdc mode [DRAFT DO NOT MERGE]~~ [Source-MsSQL] Set default cursor for cdc mode Aug 16, 2023

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Aug 16, 2023

prateekmukhedkar reviewed Aug 22, 2023

View reviewed changes

akashkulk reviewed Aug 22, 2023

View reviewed changes

akashkulk approved these changes Aug 22, 2023

View reviewed changes

akashkulk requested changes Aug 22, 2023

View reviewed changes

nguyenaiden force-pushed the sql-server-cdc-cursor branch from bc73332 to 06d2655 Compare August 22, 2023 20:50

octavia-squidington-iii added the connectors/source/mssql-strict-encrypt label Aug 22, 2023

nguyenaiden force-pushed the sql-server-cdc-cursor branch from 06d2655 to 06952f3 Compare August 22, 2023 21:59

nguyenaiden requested review from akashkulk and prateekmukhedkar August 22, 2023 23:16

akashkulk approved these changes Aug 22, 2023

View reviewed changes

nguyenaiden added 7 commits August 23, 2023 10:10

Set default cursor for cdc mode

0f8c6f8

Update tests to account for default cursor

fa06957

Set default cursor before adding metadata columns

86673bc

Bumped version + change log

f59d4b6

bumped strict encrypt version

7648b3a

Added migration guide Added breaking changes key to metadata

Updated name to match migration doc format

e132d60

Update mssql-migrations.md headers to match with expected title

34d6a46

nguyenaiden force-pushed the sql-server-cdc-cursor branch from c23dcb3 to 34d6a46 Compare August 23, 2023 17:11

nguyenaiden mentioned this pull request Aug 23, 2023

[Source-MsSQL] Rollback to 1.1.1 in case 2.0.0 fails [DO NOT MERGE] #29770

Closed

octavia-approvington approved these changes Aug 23, 2023

View reviewed changes

octavia-approvington merged commit f68a828 into master Aug 23, 2023
19 checks passed

octavia-approvington deleted the sql-server-cdc-cursor branch August 23, 2023 19:01

harrytou pushed a commit to KYVENetwork/airbyte that referenced this pull request Sep 1, 2023

[Source-MsSQL] Set default cursor for cdc mode (airbytehq#29493)

2f53fe1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Source-MsSQL] Set default cursor for cdc mode #29493

[Source-MsSQL] Set default cursor for cdc mode #29493

nguyenaiden commented Aug 16, 2023 •

edited

github-actions bot commented Aug 16, 2023 •

edited by nguyenaiden

octavia-squidington-iii commented Aug 16, 2023

octavia-squidington-iii commented Aug 16, 2023

octavia-squidington-iii commented Aug 16, 2023

nguyenaiden commented Aug 16, 2023 •

edited by github-actions bot

octavia-squidington-iii commented Aug 16, 2023

octavia-squidington-iii commented Aug 16, 2023

prateekmukhedkar left a comment

akashkulk Aug 22, 2023

nguyenaiden Aug 22, 2023

akashkulk Aug 22, 2023

nguyenaiden Aug 22, 2023

akashkulk left a comment

akashkulk left a comment

nguyenaiden commented Aug 22, 2023

github-actions bot commented Aug 22, 2023

github-actions bot commented Aug 23, 2023

nguyenaiden commented Aug 23, 2023

nguyenaiden commented Aug 23, 2023

octavia-approvington commented Aug 23, 2023

github-actions bot commented Aug 23, 2023

github-actions bot commented Aug 23, 2023

github-actions bot commented Aug 23, 2023

github-actions bot commented Aug 23, 2023

[Source-MsSQL] Set default cursor for cdc mode #29493

[Source-MsSQL] Set default cursor for cdc mode #29493

Conversation

nguyenaiden commented Aug 16, 2023 • edited

What

How

Recommended reading order

🚨 User Impact 🚨

github-actions bot commented Aug 16, 2023 • edited by nguyenaiden

Before Merging a Connector Pull Request

octavia-squidington-iii commented Aug 16, 2023

source-mssql test report (commit c6f9dee920) - ❌

octavia-squidington-iii commented Aug 16, 2023

source-mssql test report (commit c6f9dee920) - ❌

octavia-squidington-iii commented Aug 16, 2023

source-mssql test report (commit b056c83f60) - ❌

nguyenaiden commented Aug 16, 2023 • edited by github-actions bot

Build Failed

octavia-squidington-iii commented Aug 16, 2023

source-mssql test report (commit 67e3ec3cc1) - ❌

octavia-squidington-iii commented Aug 16, 2023

source-mssql test report (commit bc733328b1) - ❌

prateekmukhedkar left a comment

Choose a reason for hiding this comment

akashkulk Aug 22, 2023

Choose a reason for hiding this comment

nguyenaiden Aug 22, 2023

Choose a reason for hiding this comment

akashkulk Aug 22, 2023

Choose a reason for hiding this comment

nguyenaiden Aug 22, 2023

Choose a reason for hiding this comment

akashkulk left a comment

Choose a reason for hiding this comment

akashkulk left a comment

Choose a reason for hiding this comment

nguyenaiden commented Aug 22, 2023

github-actions bot commented Aug 22, 2023

source-mssql test report (commit 06d2655046) - ❌

github-actions bot commented Aug 23, 2023

source-mssql test report (commit f964644809) - ❌

nguyenaiden commented Aug 23, 2023

nguyenaiden commented Aug 23, 2023

octavia-approvington commented Aug 23, 2023

github-actions bot commented Aug 23, 2023

source-mssql-strict-encrypt test report (commit c23dcb3513) - ❌

github-actions bot commented Aug 23, 2023

source-mssql test report (commit c23dcb3513) - ❌

github-actions bot commented Aug 23, 2023

source-mssql test report (commit 46a8fd444e) - ❌

github-actions bot commented Aug 23, 2023

source-mssql-strict-encrypt test report (commit 46a8fd444e) - ❌

nguyenaiden commented Aug 16, 2023 •

edited

github-actions bot commented Aug 16, 2023 •

edited by nguyenaiden

source-mssql test report (commit `c6f9dee920`) - ❌

source-mssql test report (commit `c6f9dee920`) - ❌

source-mssql test report (commit `b056c83f60`) - ❌

nguyenaiden commented Aug 16, 2023 •

edited by github-actions bot

source-mssql test report (commit `67e3ec3cc1`) - ❌

source-mssql test report (commit `bc733328b1`) - ❌

source-mssql test report (commit `06d2655046`) - ❌

source-mssql test report (commit `f964644809`) - ❌

source-mssql-strict-encrypt test report (commit `c23dcb3513`) - ❌

source-mssql test report (commit `c23dcb3513`) - ❌

source-mssql test report (commit `46a8fd444e`) - ❌

source-mssql-strict-encrypt test report (commit `46a8fd444e`) - ❌