Snowflake Destination apply buffering strategy on GCS staging by VitaliiMaltsev · Pull Request #12952 · airbytehq/airbyte

VitaliiMaltsev · 2022-05-18T11:14:35Z

What

Follow buffering strategy (compressed serialized records) for Destination Snowflake GCS staging

How

This PR switches the Snowflake destination and stop using the SnowflakeCopyGcsDestination, but use a new SnowflakeGCSStagingDestination

So this PR simply requires its own set of StagingOperations using GCS buckets

🚨 User Impact 🚨

Potentially a bit slower sync but more consistent memory and thread consumption thanks to on-disk buffering before staging. (More resilient to OOM and network failures)

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

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.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
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.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

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.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

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:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

VitaliiMaltsev · 2022-05-19T08:09:05Z

/test connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/2350597498
❌ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/2350597498
🐛 https://gradle.com/s/274ccjxnh2gqy

Yurii Bidiuk (yurii-bidiuk) · 2022-05-19T15:33:24Z

Looks good to me! Let's fix a test and proceed with PR

VitaliiMaltsev · 2022-05-20T10:07:56Z

/test connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/2357594345
✅ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/2357594345
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 13      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    13      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     155      8    95%
normalization/transform_config/transform.py                         159     31    81%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_catalog/utils.py                             34      7    79%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 534    345    35%
-------------------------------------------------------------------------------------
TOTAL                                                              1320    550    58%

…ffering # Conflicts: # docs/integrations/destinations/snowflake.md

Greg Solovyev (grishick)

This PR does not compile. CI shows the following build failures and I am seeing the same compilation errors when I check out these changes. Are there more changes to more files that should be included in this PR?
> Task :airbyte-integrations:connectors:destination-snowflake:compileJava FAILED /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeGCSStagingDestination.java:57: error: incompatible types: try-with-resources not applicable to variable type try (final JdbcDatabase database = getDatabase(config)) { ^ (JdbcDatabase cannot be converted to AutoCloseable) /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeGCSStagingDestination.java:92: error: method does not override or implement a method from a supertype @Override ^ /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeGCSStagingDestination.java:94: error: incompatible types: JsonNode cannot be converted to DataSource return SnowflakeDatabase.getDatabase(config); ^

…ffering # Conflicts: # docs/integrations/destinations/snowflake.md

VitaliiMaltsev · 2022-05-25T18:55:28Z

This PR does not compile. CI shows the following build failures and I am seeing the same compilation errors when I check out these changes. Are there more changes to more files that should be included in this PR? > Task :airbyte-integrations:connectors:destination-snowflake:compileJava FAILED /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeGCSStagingDestination.java:57: error: incompatible types: try-with-resources not applicable to variable type try (final JdbcDatabase database = getDatabase(config)) { ^ (JdbcDatabase cannot be converted to AutoCloseable) /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeGCSStagingDestination.java:92: error: method does not override or implement a method from a supertype @Override ^ /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeGCSStagingDestination.java:94: error: incompatible types: JsonNode cannot be converted to DataSource return SnowflakeDatabase.getDatabase(config); ^

fixed

Greg Solovyev (grishick) · 2022-05-25T23:05:30Z

Hm... looks like CI tests maybe timing out.

Greg Solovyev (grishick) · 2022-05-25T23:44:40Z

/test connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/2387767353
❌ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/2387767353
🐛 https://gradle.com/s/2pcktgxgcywic

Build Failed

Test summary info:

Could not find result summary

Liren Tu (tuliren)

Looks good in general.

One issue is that the new implementation has lots of similar code as SnowflakeS3StagingDestination and S3StorageOperations. Ideally we would merge the shared code if possible. But this can be a separate issue.

Can you run Java formatting before merging and commit the formatting updates related to the classes in this PR? The command is: ./gradlew :spotlessJavaApply

Greg Solovyev (grishick) · 2022-05-27T00:07:58Z

After this change SnowflakeCopyGcsDestination will have no references to it in the rest of the code base, so I think this change should also delete SnowflakeCopyGcsDestination to avoid leaving behind dead code.

VitaliiMaltsev · 2022-05-29T12:27:19Z

SnowflakeCopyGcsDestination

removed SnowflakeCopyGcsDestination class

…ffering # Conflicts: # airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeCopyGcsDestination.java

VitaliiMaltsev · 2022-05-30T13:04:47Z

/publish connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/2409227308
🚀 Successfully published connectors/destination-snowflake
🚀 Auto-bumped version for connectors/destination-snowflake
✅ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/2409227308

VitaliiMaltsev · 2022-05-30T14:43:05Z

One issue is that the new implementation has lots of similar code as SnowflakeS3StagingDestination and S3StorageOperations. Ideally we would merge the shared code if possible. But this can be a separate issue.

Liren Tu (@tuliren) created separate issue https://github.com/airbytehq/airbyte-internal-issues/issues/650 because It is better to start working on it after https://github.com/airbytehq/airbyte-internal-issues/issues/501 to cover the Azure blob storage implementation as well

VitaliiMaltsev · 2022-05-30T14:45:58Z

Greg Solovyev (@grishick) could you please approve this PR cause merging is blocked currently

Greg Solovyev (grishick) · 2022-06-01T01:33:46Z

/test connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/2418754117
✅ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/2418754117
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 13      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    13      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     155      8    95%
normalization/transform_config/transform.py                         159     31    81%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_catalog/utils.py                             38      9    76%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 543    352    35%
-------------------------------------------------------------------------------------
TOTAL                                                              1333    559    58%

Build Passed

Test summary info:

All Passed

…ehq#12952) * Destination Snowflake apply buffering strategy for GCS * Snowflake Destination apply buffering strategy on GCS staging * updated CHANGELOG * fixed checkstyle * fixed compile error * refactoring * bump version * auto-bump connector version Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

VitaliiMaltsev added 2 commits May 16, 2022 12:00

Destination Snowflake apply buffering strategy for GCS

858c4ac

Snowflake Destination apply buffering strategy on GCS staging

8ece860

github-actions Bot added the area/connectors Connector related issues label May 18, 2022

updated CHANGELOG

383cf1e

github-actions Bot added the area/documentation Improvements or additions to documentation label May 18, 2022

fixed checkstyle

1c2e34f

VitaliiMaltsev requested review from Alexander Tsukanov (alexandertsukanov) and Yurii Bidiuk (yurii-bidiuk) May 18, 2022 15:11

Yurii Bidiuk (yurii-bidiuk) approved these changes May 19, 2022

View reviewed changes

Merge branch 'master' into vmaltsev/11428-dest-snowflake-gcs-apply-bu…

dfa2830

…ffering # Conflicts: # docs/integrations/destinations/snowflake.md

VitaliiMaltsev requested a review from Liren Tu (tuliren) May 23, 2022 08:23

VitaliiMaltsev linked an issue May 24, 2022 that may be closed by this pull request

Apply buffering changes to Snowflake Destination using GCS staging #11428

Closed

VitaliiMaltsev marked this pull request as ready for review May 25, 2022 07:53

Greg Solovyev (grishick) suggested changes May 25, 2022

View reviewed changes

VitaliiMaltsev added 2 commits May 25, 2022 21:28

fixed compile error

6529c9c

Merge branch 'master' into vmaltsev/11428-dest-snowflake-gcs-apply-bu…

a9c34aa

…ffering # Conflicts: # docs/integrations/destinations/snowflake.md

VitaliiMaltsev requested a review from Greg Solovyev (grishick) May 25, 2022 18:55

Liren Tu (tuliren) approved these changes May 26, 2022

View reviewed changes

refactoring

96f413d

VitaliiMaltsev added 2 commits May 29, 2022 15:28

Merge branch 'master' into vmaltsev/11428-dest-snowflake-gcs-apply-bu…

e506d3a

…ffering # Conflicts: # airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeCopyGcsDestination.java

bump version

1eddfc6

auto-bump connector version

3ef1b8c

Octavia Squidington III (octavia-squidington-iii) temporarily deployed to more-secrets May 30, 2022 14:06 Inactive

Greg Solovyev (grishick) approved these changes Jun 1, 2022

View reviewed changes

VitaliiMaltsev merged commit 748291b into master Jun 1, 2022

VitaliiMaltsev deleted the vmaltsev/11428-dest-snowflake-gcs-apply-buffering branch June 1, 2022 07:28

Octavia Squidington III (octavia-squidington-iii) mentioned this pull request Jun 2, 2022

Bump Airbyte version from 0.39.7-alpha to 0.39.8-alpha #13402

Merged

Anatolii Yatsuk (tolik0) mentioned this pull request Jul 8, 2026

feat(source-salesforce): add preserve_na_values toggle to keep 'NA'-like strings in Bulk API syncs #80738

Merged

Uh oh!

Conversation

VitaliiMaltsev commented May 18, 2022 • edited by grishick Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

Uh oh!

VitaliiMaltsev commented May 19, 2022 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yurii Bidiuk (yurii-bidiuk) commented May 19, 2022

Uh oh!

VitaliiMaltsev commented May 20, 2022 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Greg Solovyev (grishick) left a comment

Choose a reason for hiding this comment

Uh oh!

VitaliiMaltsev commented May 25, 2022

Uh oh!

Greg Solovyev (grishick) commented May 25, 2022

Uh oh!

Greg Solovyev (grishick) commented May 25, 2022 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

Uh oh!

Liren Tu (tuliren) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Greg Solovyev (grishick) commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VitaliiMaltsev commented May 29, 2022

Uh oh!

VitaliiMaltsev commented May 30, 2022 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VitaliiMaltsev commented May 30, 2022

Uh oh!

VitaliiMaltsev commented May 30, 2022

Uh oh!

Greg Solovyev (grishick) commented Jun 1, 2022 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

VitaliiMaltsev commented May 18, 2022 •

edited by grishick

Loading

VitaliiMaltsev commented May 19, 2022 •

edited by github-actions Bot

Loading

VitaliiMaltsev commented May 20, 2022 •

edited by github-actions Bot

Loading

Greg Solovyev (grishick) commented May 25, 2022 •

edited by github-actions Bot

Loading

Greg Solovyev (grishick) commented May 27, 2022 •

edited

Loading

VitaliiMaltsev commented May 30, 2022 •

edited by github-actions Bot

Loading

Greg Solovyev (grishick) commented Jun 1, 2022 •

edited by github-actions Bot

Loading