Redshift Destination Apply buffering strategy #12601

VitaliiMaltsev · 2022-05-05T08:07:16Z

What

Follow buffering strategy (compressed serialized records) for Destination Redshift

How

This PR switches the Redshift destination and stop using the RedshiftCopyS3Destination, but use a new RedshiftS3StagingDestination

So this PR simply requires its own set of StagingOperations using S3 buckets (external staging method)

🚨 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.

…buffering # Conflicts: # airbyte-integrations/connectors/destination-s3/src/main/java/io/airbyte/integrations/destination/s3/S3StorageOperations.java

VitaliiMaltsev · 2022-05-05T12:33:48Z

/test connector=connectors/destination-redshift

🕑 connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/2275689331
✅ connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/2275689331
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%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/models/airbyte_protocol.py     144      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/models/__init__.py               1      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/__init__.py                      2      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                                                                                                                            1467    550    63%

CLAassistant · 2022-05-05T13:06:43Z

All committers have signed the CLA.

…buffering

alexandertsukanov · 2022-05-05T14:17:08Z

...src/main/java/io/airbyte/integrations/destination/redshift/RedshiftStagingS3Destination.java

+        isPurgeStagingData(config));
+  }
+
+  private static boolean isPurgeStagingData(final JsonNode config) {


Why do we need this as static?

removed static

… to StagingConsumerFactory.java.

…ift-apply-buffering' into vmaltsev/11426-destination-redshift-apply-buffering

alexandertsukanov

No more comments. LGTM.

suhomud · 2022-05-05T16:27:06Z

...n-jdbc/src/main/java/io/airbyte/integrations/destination/staging/StagingConsumerFactory.java

@@ -213,7 +212,7 @@ private OnCloseFunction onCloseFunction(final JdbcDatabase database,
          }
          queryList.add(stagingOperations.copyTableQuery(database, schemaName, srcTableName, dstTableName));
        }
-
+        stagingOperations.onDestinationCloseOperations(database, writeConfigs.stream().map(WriteConfig::getOutputSchemaName).collect(Collectors.toSet()));


Not necessary but maybe shift this part writeConfigs.stream().map(WriteConfig::getOutputSchemaName).collect(Collectors.toSet()) in a separate method with name which describe what we do here

The description of this interface is located in the interface by itsself, please see
@suhomud

grishick · 2022-05-09T19:46:42Z

...redshift/src/main/java/io/airbyte/integrations/destination/redshift/RedshiftDestination.java

@@ -42,7 +42,7 @@ public static DestinationType getTypeFromConfig(final JsonNode config) {
  public static Map<DestinationType, Destination> getTypeToDestination() {
    return Map.of(
        DestinationType.INSERT_WITH_SUPER_TMP_TYPE, new RedshiftInsertDestination(RedshiftDataTmpTableMode.SUPER),
-        DestinationType.COPY_S3_WITH_SUPER_TMP_TYPE, new RedshiftCopyS3Destination(RedshiftDataTmpTableMode.SUPER));


After this change RedshiftCopyS3Destination is not going to be used anywhere and should be deleted.

After this change RedshiftCopyS3Destination is not going to be used anywhere and should be deleted.

deleted RedshiftCopyS3Destination

grishick

another minor comment: description of RedshiftDestination class needs to be updated. Currently, it refers to RedshiftCopyS3Destination, which is essentially getting deprecated with this change.

grishick · 2022-05-09T23:36:01Z

@VitaliiMaltsev , could you pease sign the CLA (see comment from CLA above)

…buffering # Conflicts: # airbyte-integrations/connectors/destination-redshift/src/main/java/io/airbyte/integrations/destination/redshift/RedshiftInsertDestination.java

VitaliiMaltsev · 2022-05-10T09:18:54Z

/test connector=connectors/destination-redshift

🕑 connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/2299673546
✅ connectors/destination-redshift https://github.com/airbytehq/airbyte/actions/runs/2299673546
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%

VitaliiMaltsev · 2022-05-10T10:12:02Z

another minor comment: description of RedshiftDestination class needs to be updated. Currently, it refers to RedshiftCopyS3Destination, which is essentially getting deprecated with this change.

updated

VitaliiMaltsev · 2022-05-10T10:12:26Z

@VitaliiMaltsev , could you pease sign the CLA (see comment from CLA above)

done

grishick · 2022-05-11T15:36:20Z

LGTM, feel free to publish the new version (🙏 that publish command works from the first attempt)

VitaliiMaltsev · 2022-05-12T08:51:29Z

/publish connector=connectors/destination-redshift

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

* Redshift Destination: Apply buffering strategy * add manifest uploading * refactoring * fixed checkstyle * updated CHANGELOG * removed redundant static * airbyte-12265: Added stagingOperations.onDestinationCloseOperations() to StagingConsumerFactory.java. * airbyte-12265: Created operations and copiers java packages. * safe delete of RedshiftCopyS3Destination.java * rename tests * bump version * auto-bump connector version Co-authored-by: Oleksandr Tsukanov <[email protected]> Co-authored-by: Octavia Squidington III <[email protected]>

VitaliiMaltsev added 2 commits May 4, 2022 11:11

Redshift Destination: Apply buffering strategy

2d805f0

add manifest uploading

dcdf673

github-actions bot added the area/connectors Connector related issues label May 5, 2022

VitaliiMaltsev added 4 commits May 5, 2022 11:08

refactoring

d058c9c

Merge branch 'master' into vmaltsev/11426-destination-redshift-apply-…

931d891

…buffering # Conflicts: # airbyte-integrations/connectors/destination-s3/src/main/java/io/airbyte/integrations/destination/s3/S3StorageOperations.java

fixed checkstyle

756d43b

updated CHANGELOG

3fc748c

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

VitaliiMaltsev requested review from alexandertsukanov and suhomud May 5, 2022 13:55

Merge branch 'master' into vmaltsev/11426-destination-redshift-apply-…

722e173

…buffering

alexandertsukanov reviewed May 5, 2022

View reviewed changes

VitaliiMaltsev and others added 2 commits May 5, 2022 17:34

removed redundant static

56b3910

airbyte-12265: Added stagingOperations.onDestinationCloseOperations()…

d9af45e

… to StagingConsumerFactory.java.

VitaliiMaltsev requested a review from alexandertsukanov May 5, 2022 14:35

Merge remote-tracking branch 'origin/vmaltsev/11426-destination-redsh…

5e3f279

…ift-apply-buffering' into vmaltsev/11426-destination-redshift-apply-buffering

alexandertsukanov approved these changes May 5, 2022

View reviewed changes

airbyte-12265: Created operations and copiers java packages.

877917f

suhomud approved these changes May 5, 2022

View reviewed changes

VitaliiMaltsev requested a review from grishick May 5, 2022 17:41

VitaliiMaltsev marked this pull request as ready for review May 5, 2022 17:41

alexandertsukanov mentioned this pull request May 6, 2022

All Syncs with redshift destination 0.3.32 fail rollback to version 0.3.28 resolves the issue #12265

Closed

VitaliiMaltsev mentioned this pull request May 9, 2022

Destination Redshift: improve check performance #11580

Closed

alexandr-shegeda linked an issue May 9, 2022 that may be closed by this pull request

Apply buffering changes to Redshift Destination #11426

Closed

grishick reviewed May 9, 2022

View reviewed changes

grishick suggested changes May 9, 2022

View reviewed changes

This was referenced May 9, 2022

Add an option to use encryption with staging in Redshift Destination #12708

Closed

Add "Loading Method" option to Redshift Destination spec #12709

Closed

VitaliiMaltsev added 3 commits May 10, 2022 11:53

Merge branch 'master' into vmaltsev/11426-destination-redshift-apply-…

162d983

…buffering # Conflicts: # airbyte-integrations/connectors/destination-redshift/src/main/java/io/airbyte/integrations/destination/redshift/RedshiftInsertDestination.java

safe delete of RedshiftCopyS3Destination.java

c0f1aba

rename tests

57eab3a

VitaliiMaltsev requested a review from grishick May 10, 2022 10:12

grishick approved these changes May 11, 2022

View reviewed changes

bump version

e657f9d

auto-bump connector version

1f661a2

octavia-squidington-iii temporarily deployed to more-secrets May 12, 2022 09:21 Inactive

VitaliiMaltsev merged commit 4b5bd2b into master May 12, 2022

VitaliiMaltsev deleted the vmaltsev/11426-destination-redshift-apply-buffering branch May 12, 2022 09:30

octavia-squidington-iii mentioned this pull request May 13, 2022

Bump Airbyte version from 0.38.2-alpha to 0.38.3-alpha #12839

Merged

ChristopheDuong mentioned this pull request May 30, 2022

Destination Redshift: throws OOM on large dataset #11695

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redshift Destination Apply buffering strategy #12601

Redshift Destination Apply buffering strategy #12601

VitaliiMaltsev commented May 5, 2022 •

edited

Loading

VitaliiMaltsev commented May 5, 2022 •

edited by github-actions bot

Loading

CLAassistant commented May 5, 2022 •

edited

Loading

alexandertsukanov May 5, 2022

VitaliiMaltsev May 5, 2022

alexandertsukanov left a comment

suhomud May 5, 2022

alexandertsukanov May 11, 2022

grishick May 9, 2022

VitaliiMaltsev May 10, 2022

grishick left a comment

grishick commented May 9, 2022

VitaliiMaltsev commented May 10, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented May 10, 2022

VitaliiMaltsev commented May 10, 2022

grishick commented May 11, 2022

VitaliiMaltsev commented May 12, 2022 •

edited by github-actions bot

Loading

Redshift Destination Apply buffering strategy #12601

Redshift Destination Apply buffering strategy #12601

Conversation

VitaliiMaltsev commented May 5, 2022 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

VitaliiMaltsev commented May 5, 2022 • edited by github-actions bot Loading

CLAassistant commented May 5, 2022 • edited Loading

alexandertsukanov May 5, 2022

Choose a reason for hiding this comment

VitaliiMaltsev May 5, 2022

Choose a reason for hiding this comment

alexandertsukanov left a comment

Choose a reason for hiding this comment

suhomud May 5, 2022

Choose a reason for hiding this comment

alexandertsukanov May 11, 2022

Choose a reason for hiding this comment

grishick May 9, 2022

Choose a reason for hiding this comment

VitaliiMaltsev May 10, 2022

Choose a reason for hiding this comment

grishick left a comment

Choose a reason for hiding this comment

grishick commented May 9, 2022

VitaliiMaltsev commented May 10, 2022 • edited by github-actions bot Loading

VitaliiMaltsev commented May 10, 2022

VitaliiMaltsev commented May 10, 2022

grishick commented May 11, 2022

VitaliiMaltsev commented May 12, 2022 • edited by github-actions bot Loading

VitaliiMaltsev commented May 5, 2022 •

edited

Loading

VitaliiMaltsev commented May 5, 2022 •

edited by github-actions bot

Loading

CLAassistant commented May 5, 2022 •

edited

Loading

VitaliiMaltsev commented May 10, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented May 12, 2022 •

edited by github-actions bot

Loading