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

Improve normalization close error message #19829

Merged
merged 19 commits into from
Nov 29, 2022

Conversation

jdpgrailsdev
Copy link
Contributor

What

  • Improve the error message when the normalization process fails

How

  • Separate the two cases that cause an error when attempting to close the normalization process
  • Include more details in the error message

Recommended reading order

  1. DefaultNormalizationRunner.java

Tests

  • All existing tests pass
  • Project builds successfully locally

@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 28, 2022 15:50 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 28, 2022 15:50 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 28, 2022 16:20 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 28, 2022 16:20 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 28, 2022 17:47 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 28, 2022 17:48 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 28, 2022 18:32 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 28, 2022 18:33 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 28, 2022 21:28 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 28, 2022 21:28 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 15:16 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 15:16 Inactive
@jdpgrailsdev jdpgrailsdev requested a review from a team as a code owner November 29, 2022 15:56
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 15:58 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 15:58 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 16:41 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 16:42 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 17:34 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 17:34 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 18:01 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 18:02 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 19:52 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 19:52 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 19:58 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 19:58 Inactive
…ub.com:airbytehq/airbyte into jonathan/improve-normalization-close-error-msg
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 20:07 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets November 29, 2022 20:07 Inactive
@jdpgrailsdev jdpgrailsdev merged commit e06bdf6 into master Nov 29, 2022
@jdpgrailsdev jdpgrailsdev deleted the jonathan/improve-normalization-close-error-msg branch November 29, 2022 20:41
@Aggiekev
Copy link

Aggiekev commented Dec 6, 2022

I believe this merge is causing my Connections to fail during normalization. The error message isn't descriptive in the logs but this is the only merge that dealt with normalization in the last release which is where my Connections are failing. They fail at different points in the transformation for every single connection. My guess is that it is related to the waiting for one minute to throw the error?

@natalyjazzviolin
Copy link
Contributor

@edgao looks like this is causing failures for an OSS user - @Aggiekev . Their instance is K8 on GKE, logs attached below.
f62c3bbc_ade5_44cf_b1ba_96e850184f73_logs_1415_txt (1).txt

@edgao
Copy link
Contributor

edgao commented Dec 7, 2022

@jdpgrailsdev do you recognize what's happening in that sync? It looks like normalization terminates cleanly (2022-12-06 05:50:16 normalization > Completed successfully) but DefaultNormalizationWorker thinks it failed:

2022-12-06 05:50:17 INFO i.a.w.t.TemporalAttemptExecution(lambda$getWorkerThread$5):198 - Completing future exceptionally...
io.airbyte.workers.exception.WorkerException: Normalization Failed.
        at io.airbyte.workers.general.DefaultNormalizationWorker.run(DefaultNormalizationWorker.java:103) ~[io.airbyte-airbyte-commons-worker-0.40.23.jar:?]
        at io.airbyte.workers.general.DefaultNormalizationWorker.run(DefaultNormalizationWorker.java:34) ~[io.airbyte-airbyte-commons-worker-0.40.23.jar:?]
        at io.airbyte.workers.temporal.TemporalAttemptExecution.lambda$getWorkerThread$5(TemporalAttemptExecution.java:195) ~[io.airbyte-airbyte-workers-0.40.23.jar:?]
        at java.lang.Thread.run(Thread.java:1589) ~[?:?]

@jdpgrailsdev
Copy link
Contributor Author

jdpgrailsdev commented Dec 7, 2022

@jdpgrailsdev do you recognize what's happening in that sync? It looks like normalization terminates cleanly (2022-12-06 05:50:16 normalization > Completed successfully) but DefaultNormalizationWorker thinks it failed:

2022-12-06 05:50:17 INFO i.a.w.t.TemporalAttemptExecution(lambda$getWorkerThread$5):198 - Completing future exceptionally...
io.airbyte.workers.exception.WorkerException: Normalization Failed.
        at io.airbyte.workers.general.DefaultNormalizationWorker.run(DefaultNormalizationWorker.java:103) ~[io.airbyte-airbyte-commons-worker-0.40.23.jar:?]
        at io.airbyte.workers.general.DefaultNormalizationWorker.run(DefaultNormalizationWorker.java:34) ~[io.airbyte-airbyte-commons-worker-0.40.23.jar:?]
        at io.airbyte.workers.temporal.TemporalAttemptExecution.lambda$getWorkerThread$5(TemporalAttemptExecution.java:195) ~[io.airbyte-airbyte-workers-0.40.23.jar:?]
        at java.lang.Thread.run(Thread.java:1589) ~[?:?]

@edgao There was an issue introduced before this PR that caused a failure when attempting to stop/close the process. This was due to calling .pid() on the KubePodProcess, which had not implemented the method. The issue with .pid() has been fixed in a subsequent commit #20028

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants