Skip to content

MSSQL and debezium improvements - #44759

Merged
Rodi Reich Zilberman (rodireich) merged 35 commits into
masterfrom
6192-db-sources-mssql-javalangillegalargumentexception-argument-content-is-null-after-adding-primary-key
Aug 28, 2024
Merged

Rodi Reich Zilberman (rodireich) merged 35 commits into
masterfrom
6192-db-sources-mssql-javalangillegalargumentexception-argument-content-is-null-after-adding-primary-key

Conversation

@rodireich

@rodireich Rodi Reich Zilberman (rodireich) commented Aug 26, 2024 •

Copy link
Copy Markdown
Contributor

What

A few issues in debezium and mssql were discovered as a result of https://github.com/airbytehq/oncall/issues/6192 .

  1. Conversion of incoming debezium change events to ChangeEventWithMetadata was not null safe.
  2. During an incremental run of mssql, we'd run debezium with configuration that could turn into initial snapshot.
  3. We processed incoming debezium events categorized as op: r which is can only be part of an initial snapshot.

This PR deals with the discoveries and adds a new troubleshooting page for the root cause of the oncall issue.

@vercel

vercel Bot commented Aug 26, 2024 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 28, 2024 10:53pm

@octavia-squidington-iii Octavia Squidington III (octavia-squidington-iii) added the CDK Connector Development Kit label Aug 26, 2024

if (isEventLogged) {
val source: JsonNode? = changeEventWithMetadata.eventValueAsJson()["source"]
val source: JsonNode? = changeEventWithMetadata.eventValueAsJson?.get("source")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

kotlin doesn't have ?[] unfortunately

@rodireich

Copy link
Copy Markdown
Contributor Author

Evan Tahler (@evantahler) I'd like to add a troubleshooting doc for mssql, similarly to what we have in other connectors (e.g).
Is there anything I need to do other than adding the doc?

@evantahler

Copy link
Copy Markdown
Contributor

Rodi Reich Zilberman (@rodireich) - you've got to add the new doc to the sidebar nav as well. I can help if you get stuck!

@rodireich Rodi Reich Zilberman (rodireich) changed the title test MSSQL and debezium improvements Aug 28, 2024
@rodireich
Rodi Reich Zilberman (rodireich) marked this pull request as ready for review August 28, 2024 19:53
…entexception-argument-content-is-null-after-adding-primary-key

@postamar Marius Posta (postamar) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My only concern here is the potential for log spam, LGTM otherwise.

event
.value()
?.let { Jsons.deserialize(it) }
.also { it ?: LOGGER.warn { "Event value is null $event" } }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could spam the logs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we get to the situation where value or key are null then something is seriously misoncifugred, as was the case on the oncall ticket.
I'd rather know what the malformed debezium event looks like. I couldn't figure out the root cause until I was able to get to this information.

@rodireich

Rodi Reich Zilberman (rodireich) commented Aug 28, 2024 •

Copy link
Copy Markdown
Contributor Author

/publish-java-cdk dry-run=true

🕑 https://github.com/airbytehq/airbyte/actions/runs/10605240753
✅ Successfully published Java CDK version=0.44.18!

@rodireich

Rodi Reich Zilberman (rodireich) commented Aug 28, 2024 •

Copy link
Copy Markdown
Contributor Author

/publish-java-cdk

🕑 https://github.com/airbytehq/airbyte/actions/runs/10605307052
✅ Successfully published Java CDK version=0.44.18!

@rodireich
Rodi Reich Zilberman (rodireich) deleted the 6192-db-sources-mssql-javalangillegalargumentexception-argument-content-is-null-after-adding-primary-key branch August 28, 2024 23:09

This branch was successfully deployed

1 active deployment
Preview — 7230252d Deployed Aug 28, 2024 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/connectors Connector related issues area/documentation Improvements or additions to documentation CDK Connector Development Kit connectors/source/mssql

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants