[source-postgres] State counter on postgres #34724

xiaohansong · 2024-01-31T23:57:38Z

What

State count of records for postgres incremental sync

How

Describe the solution

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user?

For connector PRs, use this section to explain which type of semantic versioning bump occurs as a result of the changes. Refer to our Semantic Versioning for Connectors guidelines for more information. Breaking changes to connectors must be documented by an Airbyte engineer (PR author, or reviewer for community PRs) by using the Breaking Change Release Playbook.

If there are breaking changes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Actions

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Community member? Grant edit access to maintainers (instructions)
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Connector version is set to 0.0.1
- Dockerfile has version 0.0.1
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- docs/integrations/<source or destination>/<name>.md including changelog with an entry for the initial version. See changelog example
- docs/integrations/README.md

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Unit & integration tests added

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:generateScaffolds then checking in your changes
Documentation which references the generator is updated as needed

Updating the Python CDK

Airbyter

Before merging:

Pull Request description explains what problem it is solving
Code change is unit tested
Build and my-py check pass
Smoke test the change on at least one affected connector
- On Github: Run this workflow, passing --use-local-cdk --name=source-<connector> as options
- Locally: airbyte-ci connectors --use-local-cdk --name=source-<connector> test
PR is reviewed and approved

After merging:

Publish the CDK
- The CDK does not follow proper semantic versioning. Choose minor if this the change has significant user impact or is a breaking change. Choose patch otherwise.
- Write a thoughtful changelog message so we know what was updated.
Merge the platform PR that was auto-created for updating the Connector Builder's CDK version
- This step is optional if the change does not affect the connector builder or declarative connectors.

erge remote-tracking branch 'origin/master' into xiaohan/otheriterators

vercel · 2024-01-31T23:57:44Z

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

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	⬜️ Ignored (Inspect)	Visit Preview		Mar 7, 2024 5:33pm

github-actions · 2024-01-31T23:57:56Z

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.

github-actions · 2024-02-05T18:17:11Z

Coverage report for source-postgres

File	Coverage [93.87%]	🍏
CtidStateIteratorManager.java	96.67%	🍏
PostgresCtidHandler.java	94.8%	🍏
XminStateIteratorManager.java	87.1%	🍏
XminStateIterator.java	82.93%	🍏

Total Project Coverage	69.23%	🍏

Co-authored-by: SatishChGit <[email protected]> Co-authored-by: evantahler <[email protected]>

…34641)

…34681)

akashkulk · 2024-03-05T17:35:30Z

...cdk/db-sources/src/testFixtures/java/io/airbyte/cdk/integrations/debezium/CdcSourceTest.java

+  // TODO: this assertion should be added into test cases in this class, we will need to implement
+  // corresponding iterator for other connectors before
+  // doing so.
+  protected void assertExpectedStateMessageCountMatches(final List<AirbyteStateMessage> stateMessages, long totalCount) {


Can we enable it for the relevant paths (Postgres/MySQL CDC) and disable it for others? That way we have coverage

akashkulk · 2024-03-05T17:51:12Z

...postgres/src/main/java/io/airbyte/integrations/source/postgres/ctid/PostgresCtidHandler.java

    final Duration syncCheckpointDuration =
        config.get(SYNC_CHECKPOINT_DURATION_PROPERTY) != null ? Duration.ofSeconds(config.get(SYNC_CHECKPOINT_DURATION_PROPERTY).asLong())
-            : CtidStateIterator.SYNC_CHECKPOINT_DURATION;
+            : DebeziumIteratorConstants.SYNC_CHECKPOINT_DURATION;


nit : perhaps we should create a new InitialLoadConstants.SYNC_CHECKPOINT_DURATIONS to have the ability to differentiate between different checkpointing constants?

Is there a difference for doing initial load vs regular incremental load? Feels like we should do that only if we found that would be required.

No difference right now. I think we don't need to create additional ones. But maybe a todo or something to rename the file in DebeziumIteratorConstants to just IteratorConstants

added a to do. Will move after I checked in other iterators.

akashkulk · 2024-03-05T18:10:29Z

...e-postgres/src/main/java/io/airbyte/integrations/source/postgres/xmin/XminStateIterator.java

 import java.util.Iterator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;

-public class XminStateIterator extends AbstractIterator<AirbyteMessage> implements Iterator<AirbyteMessage> {
+public class XminStateIterator extends SourceStateIterator<AirbyteMessage> {


Remind me again : Why can't we get rid of XminStateIterator like we did the CtidStateIterator? We should try to get rid of the individual state iterators if possible

mostly because of the error handling - if there's exception, xminStateIterator will not throw but just end gracefully. (Is that still expected?)

akashkulk

Some minor comments here

xiaohansong · 2024-03-07T17:17:31Z

/publish-java-cdk

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

xiaohansong added 10 commits December 11, 2023 10:20

add comment on interface

f8b2b84

delete unrelated merge change

27e3713

add unit test for sourceStateIterator

f9a270e

cdk change - support ctid

e65713f

mysql related change, and support ctid

34999c7

save work

140246e

:wq

4be3cc8

erge remote-tracking branch 'origin/master' into xiaohan/otheriterators

merge

2f3c0c6

revert unnecessary changes

78e2fff

xmin tested

f7efbe8

octavia-squidington-iii added area/connectors Connector related issues connectors/source/postgres labels Jan 31, 2024

add/edit test for xmin

86e59a5

aaronsteers and others added 14 commits February 13, 2024 13:59

Source Faker: Declare primary keys (#34644)

e5cc585

Destination Redshift - Bump CDK version to 0.16.3 (#34680)

e31c6d4

Destination Teradata: make connector avaialble on Airbyte Cloud (#28667)

592040b

Co-authored-by: SatishChGit <[email protected]> Co-authored-by: evantahler <[email protected]>

Support resuming initial snapshot when id type is String, Int, Long (#…

cef20c1

…34641)

Publish to pypi batch4 (#34666)

901be76

airbyte-ci: Test pypi published properly (#34689)

04ce62c

Publish to pypi batch5 (#34668)

0043b0d

Publish to pypi batch6 (#34672)

9c4a67d

Publish to pypi batch7 (#34673)

2eb8fc6

Kubernetes docs: update instructions to use external database (#34604)

aa8a439

Update file-based connectors for compatibility with concurrent CDK (#…

5e63c11

…34681)

🚨🚨🐛Source Hubspot: update marketing_emails stream schema (#34492)

b05031a

Publish to pypi batch8 (#34690)

e9d9b6d

Source Azure Table Storage: CDK Update (#34576)

7443c5c

xiaohansong added 4 commits February 27, 2024 11:29

merge state manager together

d66ef87

Merge branch 'master' into xiaohan/ctiditerator

75d3d97

postgres use stream in iterator

4db2b89

Merge remote-tracking branch 'origin/master' into xiaohan/ctiditerator

2bc1fcb

vercel bot deployed to Preview February 29, 2024 18:39 View deployment

xiaohansong added 2 commits February 29, 2024 10:54

conform to new format, and auto-formatting

a62850a

format, bump

ca3da03

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Feb 29, 2024

vercel bot deployed to Preview February 29, 2024 19:18 View deployment

akashkulk reviewed Mar 5, 2024

View reviewed changes

akashkulk suggested changes Mar 5, 2024

View reviewed changes

xiaohansong added 3 commits March 5, 2024 10:52

more test

6a6533f

xmin iterator follow state iterator

5d51662

add todo

dd8e352

xiaohansong requested a review from akashkulk March 5, 2024 22:41

akashkulk approved these changes Mar 5, 2024

View reviewed changes

Merge branch 'master' into xiaohan/ctiditerator

2f04c55

vercel bot deployed to Preview March 6, 2024 00:53 View deployment

xiaohansong added 5 commits March 6, 2024 14:32

Merge remote-tracking branch 'origin/master' into xiaohan/ctiditerator

e87adbf

bypass deprecation check to uncover other issues

c82d61a

extend test for postgres to 5m

ced4a8c

Merge remote-tracking branch 'origin/master' into xiaohan/ctiditerator

e7073cf

upgrade cdk

ddcda7b

upgrade

d0ecd23

xiaohansong enabled auto-merge (squash) March 7, 2024 17:42

xiaohansong merged commit 1bb7a1c into master Mar 7, 2024

xiaohansong deleted the xiaohan/ctiditerator branch March 7, 2024 17:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[source-postgres] State counter on postgres #34724

[source-postgres] State counter on postgres #34724

xiaohansong commented Jan 31, 2024

vercel bot commented Jan 31, 2024 •

edited

Loading

github-actions bot commented Jan 31, 2024 •

edited by xiaohansong

Loading

github-actions bot commented Feb 5, 2024

akashkulk Mar 5, 2024

xiaohansong Mar 5, 2024

akashkulk Mar 5, 2024

xiaohansong Mar 5, 2024

akashkulk Mar 5, 2024

xiaohansong Mar 5, 2024

akashkulk Mar 5, 2024

xiaohansong Mar 5, 2024

akashkulk left a comment

xiaohansong commented Mar 7, 2024 •

edited by github-actions bot

Loading

[source-postgres] State counter on postgres #34724

[source-postgres] State counter on postgres #34724

Conversation

xiaohansong commented Jan 31, 2024

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Actions

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Airbyter

vercel bot commented Jan 31, 2024 • edited Loading

github-actions bot commented Jan 31, 2024 • edited by xiaohansong Loading

Before Merging a Connector Pull Request

github-actions bot commented Feb 5, 2024

Coverage report for source-postgres

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akashkulk left a comment

Choose a reason for hiding this comment

xiaohansong commented Mar 7, 2024 • edited by github-actions bot Loading

vercel bot commented Jan 31, 2024 •

edited

Loading

github-actions bot commented Jan 31, 2024 •

edited by xiaohansong

Loading

xiaohansong commented Mar 7, 2024 •

edited by github-actions bot

Loading