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.

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

Add ExitOnOutOfMemoryError to java connectors and bump versions #10256

Merged

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

/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

/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

/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
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

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

/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

/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

/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

/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

/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

/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

/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

/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

/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

/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

/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
Copy link
Contributor

tuliren commented Feb 13, 2022

@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

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

/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
@tuliren tuliren mentioned this pull request Feb 15, 2022
@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants