Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ExitOnOutOfMemoryError to java connectors and bump versions #10256

Merged
merged 9 commits into from
Feb 14, 2022
Merged

Add ExitOnOutOfMemoryError to java connectors and bump versions #10256

merged 9 commits into from
Feb 14, 2022

Conversation

lmossman
Copy link
Contributor

See context here: https://airbytehq-team.slack.com/archives/C02U46QK5C3/p1644534663479069?thread_ts=1644532106.854279&cid=C02U46QK5C3

We have connectors that are hanging due to the java thread throwing an OutOfMemoryError, but the pod not actually failing. Adding this flag will cause the process to exit, correctly closing the pod and surfacing the OOM issue faster.

@github-actions github-actions bot added area/connectors Connector related issues area/platform issues related to the platform area/scheduler area/server area/worker Related to worker labels Feb 11, 2022
@lmossman lmossman temporarily deployed to more-secrets February 11, 2022 00:19 Inactive
@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/destination-azure-blob-storage

🕑 connectors/destination-azure-blob-storage https://github.com/airbytehq/airbyte/actions/runs/1826882767
✅ connectors/destination-azure-blob-storage https://github.com/airbytehq/airbyte/actions/runs/1826882767
No Python unittests run

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/destination-gcs

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

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

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/1826886192
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/1826886192
No Python unittests run

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 11, 2022 00:25 Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 11, 2022 00:25 Inactive
@lmossman lmossman requested a review from edgao February 11, 2022 00:26
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 11, 2022 00:26 Inactive
edgao
edgao approved these changes Feb 11, 2022
View reviewed changes
Copy link
Contributor

@edgao edgao left a comment

Choose a reason for hiding this comment

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

:shipit:

not sure why the gcs test failed. will try to repro locally, but I don't think it blocks this PR

cgardens
cgardens approved these changes Feb 11, 2022
View reviewed changes
Copy link
Contributor

@cgardens cgardens left a comment

Choose a reason for hiding this comment

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

Clutch!

Seeing the scope of this PR makes it feel like we should figure out how to inherit these JVM options across connectors! Definitely not a right now thing. Just an observation.

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 11, 2022 00:45 Inactive
@lmossman
Copy link
Contributor Author

@cgardens there is sort of an existing issue to take care of that, which was created when we added the max RAM percentage flag originally: https://github.com/airbytehq/airbyte-internal-issues/issues/264

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-azure-blob-storage

🕑 connectors/destination-azure-blob-storage https://github.com/airbytehq/airbyte/actions/runs/1827039338
✅ connectors/destination-azure-blob-storage https://github.com/airbytehq/airbyte/actions/runs/1827039338

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-bigquery-denormalized

🕑 connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/1827041552
❌ connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/1827041552

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-bigquery

🕑 connectors/destination-bigquery https://github.com/airbytehq/airbyte/actions/runs/1827042314
❌ connectors/destination-bigquery https://github.com/airbytehq/airbyte/actions/runs/1827042314

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-cassandra

🕑 connectors/destination-cassandra https://github.com/airbytehq/airbyte/actions/runs/1827043201
✅ connectors/destination-cassandra https://github.com/airbytehq/airbyte/actions/runs/1827043201

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-clickhouse-strict-encrypt

🕑 connectors/destination-clickhouse-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1827043841
❌ connectors/destination-clickhouse-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1827043841
🕑 connectors/destination-clickhouse-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1827043841
❌ connectors/destination-clickhouse-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1827043841

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-clickhouse

🕑 connectors/destination-clickhouse https://github.com/airbytehq/airbyte/actions/runs/1827044579
✅ connectors/destination-clickhouse https://github.com/airbytehq/airbyte/actions/runs/1827044579

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-csv

🕑 connectors/destination-csv https://github.com/airbytehq/airbyte/actions/runs/1827045073
✅ connectors/destination-csv https://github.com/airbytehq/airbyte/actions/runs/1827045073

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-databricks

🕑 connectors/destination-databricks https://github.com/airbytehq/airbyte/actions/runs/1827045494
✅ connectors/destination-databricks https://github.com/airbytehq/airbyte/actions/runs/1827045494

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-dev-null

🕑 connectors/destination-dev-null https://github.com/airbytehq/airbyte/actions/runs/1827045993
✅ connectors/destination-dev-null https://github.com/airbytehq/airbyte/actions/runs/1827045993

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-dynamodb

🕑 connectors/destination-dynamodb https://github.com/airbytehq/airbyte/actions/runs/1827046516
✅ connectors/destination-dynamodb https://github.com/airbytehq/airbyte/actions/runs/1827046516

@lmossman
Copy link
Contributor Author

lmossman commented Feb 11, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/destination-elasticsearch

🕑 connectors/destination-elasticsearch https://github.com/airbytehq/airbyte/actions/runs/1827046817
✅ connectors/destination-elasticsearch https://github.com/airbytehq/airbyte/actions/runs/1827046817

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 11, 2022 20:44 Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 11, 2022 20:44 Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 11, 2022 20:45 Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 11, 2022 20:45 Inactive
@edgao
Copy link
Contributor

edgao commented Feb 12, 2022

state of the world:

@tuliren tuliren mentioned this pull request Feb 13, 2022
Merged
16 tasks
@tuliren
Copy link
Contributor

tuliren commented Feb 13, 2022 •
edited
Loading

@lmossman, by the way, we no longer need to update the version in those json definition files (e.g. airbyte-config/init/src/main/resources/config/STANDARD_SOURCE_DEFINITION/435bb9a5-7887-4809-aa58-28c27df0d7ad.json). Those files still exist for backward compatibility. They are not used by the current Airbyte server any more. I will revert those changes to make the PR slightly simpler.

@tuliren tuliren temporarily deployed to more-secrets February 13, 2022 03:41 Inactive
@tuliren tuliren temporarily deployed to more-secrets February 13, 2022 03:45 Inactive
@tuliren
Copy link
Contributor

tuliren commented Feb 13, 2022 •
edited
Loading

In the future, it is probably better to merge in the changes first, and publish the connectors in separate PRs. Having one PR to update so many connectors is very messy. For example, none of the changelog is updated in the connector documentation. Also some of the connectors have been published with this PR, but the change is not merged, which can block other PRs trying to update the same connectors.

@tuliren
Copy link
Contributor

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

/publish connector=connectors/destination-gcs

🕑 connectors/destination-gcs https://github.com/airbytehq/airbyte/actions/runs/1835719832
✅ connectors/destination-gcs https://github.com/airbytehq/airbyte/actions/runs/1835719832

@tuliren tuliren temporarily deployed to more-secrets February 13, 2022 03:57 Inactive
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Feb 13, 2022
@tuliren tuliren temporarily deployed to more-secrets February 13, 2022 03:58 Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 13, 2022 04:00 Inactive
@edgao edgao merged commit 3d8a0dc into master Feb 14, 2022
@edgao edgao deleted the lmossman/add-exit-on-out-of-memory-error-to-java-connectors branch February 14, 2022 23:49
@edgao edgao temporarily deployed to more-secrets February 14, 2022 23:50 Inactive
@edgao edgao temporarily deployed to more-secrets February 14, 2022 23:50 Inactive
@edgao edgao mentioned this pull request Feb 15, 2022
Merged
@tuliren tuliren mentioned this pull request Feb 15, 2022
Merged
@cgardens
Copy link
Contributor

wahoo! thanks for pushing it across the finish line.

@cgardens
Copy link
Contributor

In the future, it is probably better to merge in the changes first, and publish the connectors in separate PRs. Having one PR to update so many connectors is very messy. For example, none of the changelog is updated in the connector documentation. Also some of the connectors have been published with this PR, but the change is not merged, which can block other PRs trying to update the same connectors.

1 . agreed with liren.

@noahkawasaki-airbyte noahkawasaki-airbyte mentioned this pull request Apr 20, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cgardens cgardens cgardens approved these changes

@edgao edgao edgao approved these changes

Assignees
No one assigned
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation area/platform issues related to the platform area/scheduler area/server area/worker Related to worker
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lmossman @edgao @tuliren @cgardens @octavia-squidington-iii