🎉 Destination snowflake: reduce memory consumption #10297

tuliren · 2022-02-12T06:28:54Z

What

Relate to [EPIC] scale warehouse destination connectors to handle arbitrary number of streams #10260.
Reduce buffer size from 256 MB to 128 MB.
Refactor buffered stream consumer to be more efficient.
- Avoid creating a new data adapter for every record message.
- Change the buffer from a list to map to avoid grouping the list by streams.

🚨 User Impact 🚨

None expected.

Pre-merge Checklist

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
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing.
New Connector version released on Dockerhub by running the /publish command described here
After the new connector version is published, connector version bumped in the seed directory 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

tuliren · 2022-02-14T18:29:24Z

...stination-jdbc/src/main/java/io/airbyte/integrations/destination/jdbc/JdbcSqlOperations.java

@@ -137,7 +128,8 @@ public final void insertRecords(final JdbcDatabase database,
      throws Exception {
    AirbyteSentry.executeWithTracing("InsertRecords",
        () -> {
-          records.forEach(airbyteRecordMessage -> getDataAdapter().adapt(airbyteRecordMessage.getData()));


This is bad. For every record message, a new data adapter object is created (in the getDataAdapter method).

obviously this is already way better, but could we just dump the data adapter into a field rather than constructing one per record batch?

Ah, good point! Done.

tuliren · 2022-02-14T22:06:45Z

/test connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/1843744731
❌ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/1843744731
🐛

edgao

lgtm!

tuliren · 2022-02-15T05:38:24Z

/test connector=connectors/destination-snowflake

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

Name                                                                                                                            Stmts   Miss  Cover
---------------------------------------------------------------------------------------------------------------------------------------------------
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/__init__.py                      2      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/models/airbyte_protocol.py     124      0   100%
normalization/__init__.py                                                                                                           4      0   100%
normalization/destination_type.py                                                                                                  13      0   100%
normalization/transform_catalog/__init__.py                                                                                         2      0   100%
normalization/transform_catalog/catalog_processor.py                                                                              143     77    46%
normalization/transform_catalog/destination_name_transformer.py                                                                   155      8    95%
normalization/transform_catalog/reserved_keywords.py                                                                               13      0   100%
normalization/transform_catalog/stream_processor.py                                                                               520    333    36%
normalization/transform_catalog/table_name_registry.py                                                                            174     34    80%
normalization/transform_catalog/transform.py                                                                                       45     26    42%
normalization/transform_catalog/utils.py                                                                                           33      7    79%
normalization/transform_config/__init__.py                                                                                          2      0   100%
normalization/transform_config/transform.py                                                                                       148     34    77%
---------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                                                            1379    519    62%

tuliren · 2022-02-15T06:52:32Z

Interesting that when I pulled in the master branch (af4da72), which includes a commit that fixes the connector base build, the build still failed, as if the fix was not working.
It's only after I pushed a new commit to this PR (d5a0c23), did the build pass.

tuliren · 2022-02-15T07:16:26Z

/publish connector=connectors/destination-snowflake run-tests=false

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/1845533217
✅ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/1845533217

github-actions bot added the area/connectors Connector related issues label Feb 12, 2022

tuliren added 4 commits February 12, 2022 00:51

Avoid redundant adapter construction

cd0cfec

Remove unused logger

3aa2389

Avoid redundant creation of buffer map

8b9a6aa

Decrease max batch byte size to 128 mb

069f610

tuliren force-pushed the liren/snowflake-memory branch from 94d47b9 to 069f610 Compare February 12, 2022 08:51

tuliren temporarily deployed to more-secrets February 12, 2022 08:52 Inactive

tuliren requested review from subodh1810, edgao and sherifnada February 14, 2022 18:24

tuliren marked this pull request as ready for review February 14, 2022 18:24

tuliren commented Feb 14, 2022

View reviewed changes

octavia-squidington-iii temporarily deployed to more-secrets February 14, 2022 22:09 Inactive

edgao approved these changes Feb 15, 2022

View reviewed changes

tuliren temporarily deployed to more-secrets February 15, 2022 04:22 Inactive

tuliren temporarily deployed to more-secrets February 15, 2022 04:30 Inactive

tuliren temporarily deployed to more-secrets February 15, 2022 04:31 Inactive

tuliren added 2 commits February 14, 2022 21:27

Format code

b2668ec

Merge branch 'master' into liren/snowflake-memory

7aaddd2

tuliren force-pushed the liren/snowflake-memory branch from 0121a61 to 7aaddd2 Compare February 15, 2022 05:27

tuliren temporarily deployed to more-secrets February 15, 2022 05:28 Inactive

Move data adapter to an instance variable

80d1f10

tuliren temporarily deployed to more-secrets February 15, 2022 05:38 Inactive

octavia-squidington-iii temporarily deployed to more-secrets February 15, 2022 05:40 Inactive

Merge branch 'master' into liren/snowflake-memory

af4da72

tuliren temporarily deployed to more-secrets February 15, 2022 05:52 Inactive

Bump version

d5a0c23

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

tuliren temporarily deployed to more-secrets February 15, 2022 06:34 Inactive

octavia-squidington-iii temporarily deployed to more-secrets February 15, 2022 07:19 Inactive

Bump version in seed

062de07

tuliren mentioned this pull request Feb 15, 2022

tweak snowflake destination to hide INSERT load method #9982

Merged

tuliren temporarily deployed to more-secrets February 15, 2022 07:36 Inactive

tuliren merged commit 6301cfa into master Feb 15, 2022

tuliren deleted the liren/snowflake-memory branch February 15, 2022 07:37

octavia-squidington-iii mentioned this pull request Feb 15, 2022

Bump Airbyte version from 0.35.29-alpha to 0.35.30-alpha #10358

Merged

tuliren mentioned this pull request Feb 16, 2022

[EPIC] scale warehouse destination connectors to handle arbitrary number of streams #10260

Open

30 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 Destination snowflake: reduce memory consumption #10297

🎉 Destination snowflake: reduce memory consumption #10297

tuliren commented Feb 12, 2022 •

edited

Loading

tuliren Feb 14, 2022

edgao Feb 15, 2022 •

edited

Loading

tuliren Feb 15, 2022

tuliren commented Feb 14, 2022 •

edited by github-actions bot

Loading

edgao left a comment

tuliren commented Feb 15, 2022 •

edited by github-actions bot

Loading

tuliren commented Feb 15, 2022 •

edited

Loading

tuliren commented Feb 15, 2022 •

edited by github-actions bot

Loading

🎉 Destination snowflake: reduce memory consumption #10297

🎉 Destination snowflake: reduce memory consumption #10297

Conversation

tuliren commented Feb 12, 2022 • edited Loading

What

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

tuliren Feb 14, 2022

Choose a reason for hiding this comment

edgao Feb 15, 2022 • edited Loading

Choose a reason for hiding this comment

tuliren Feb 15, 2022

Choose a reason for hiding this comment

tuliren commented Feb 14, 2022 • edited by github-actions bot Loading

edgao left a comment

Choose a reason for hiding this comment

tuliren commented Feb 15, 2022 • edited by github-actions bot Loading

tuliren commented Feb 15, 2022 • edited Loading

tuliren commented Feb 15, 2022 • edited by github-actions bot Loading

tuliren commented Feb 12, 2022 •

edited

Loading

edgao Feb 15, 2022 •

edited

Loading

tuliren commented Feb 14, 2022 •

edited by github-actions bot

Loading

tuliren commented Feb 15, 2022 •

edited by github-actions bot

Loading

tuliren commented Feb 15, 2022 •

edited

Loading

tuliren commented Feb 15, 2022 •

edited by github-actions bot

Loading