async container launch kubernetes "process" #9242

jrhizor · 2022-01-01T01:44:31Z

abstracts away the launcher workers with LauncherWorker
introduces AsyncOrchestratorPodProcess to "monitor" a pod that's running, so if it's re-run with the same attempt it continues with the same existing pod without killing it.

This change is

jrhizor · 2022-01-01T01:45:17Z

.env

@@ -98,3 +98,5 @@ STATE_STORAGE_GCS_BUCKET_NAME=

 # Sentry
 SENTRY_DSN="https://[email protected]/6102835"
+
+CONTAINER_ORCHESTRATOR_ENABLED=true


remove this before merging

…ch-via-cloud-storage

jrhizor · 2022-01-06T16:03:37Z

@davinchia @benmoriceau would like some first looks over the AsyncOrchestratorPodProcess and *LauncherWorker classes.

Still have some cleanup to do, just want general feedback on the approach.

benmoriceau · 2022-01-06T17:22:17Z

airbyte-workers/src/main/java/io/airbyte/workers/process/AsyncOrchestratorPodProcess.java

+      } else if (secondDocStoreStatus.equals(AsyncKubePodStatus.SUCCEEDED)) {
+        return 0;
+      } else {
+        // otherwise, the actual pod is terminal when the doc store says it shouldn't be.


Should we update the doc store as failed here?

I think it's important for the contract to be that only the container orchestrator itself writes to the store and that we handle that state + the kube cluster state instead of trusting other pieces to write to the store as well.

agree. can we document this expectation in the container orchestrator and the state manager doc strings so it's obvious to devs?

benmoriceau · 2022-01-06T17:29:18Z

airbyte-workers/src/main/java/io/airbyte/workers/process/AsyncOrchestratorPodProcess.java

+  }
+
+  @Override
+  public void destroy() {


Should we delete the doc store here

I think there's no downside to maintaining a history on cloud storage? Maybe we'll need to add this to job sweeping eventually.

Agree we can create an issue to remind ourselves and punt this till later.

airbyte-workers/src/main/java/io/airbyte/workers/process/AsyncOrchestratorPodProcess.java

airbyte-workers/src/main/java/io/airbyte/workers/temporal/sync/ReplicationLauncherWorker.java

airbyte-workers/src/main/java/io/airbyte/workers/process/AsyncKubePodStatus.java

davinchia · 2022-01-07T16:50:46Z

I think I generally follow. The pattern to poll on the api and update the state bucket seems reasonable!

I think my main concern is that there seems to be too many places where state can change e.g. the OrchestratorApp writing state. This can get complicated quickly. I think we should try to either keep all state writes/reads in a class or a clear set of abstractions or set a standard around state management e.g. state management cannot happen more than 1 abstraction layer away from the async process. This would also make things easier to test.

I'm worried because process state has been biting us a lot. Introducing a remote cache is great and can also make this more complex.

I'm not commenting on specific stuff since you said this is still WIP. Appreciate the early review request!

(Sorry I know it's quite hand wavy of me to say this. I haven't put much thought into an alternative approach yet. I'm happy to spend some time next week if needed)

…ch-via-cloud-storage

...ontainer-orchestrator/src/main/java/io/airbyte/container_orchestrator/AsyncStateManager.java

...r-orchestrator/src/main/java/io/airbyte/container_orchestrator/ContainerOrchestratorApp.java

...ntainer-orchestrator/src/main/java/io/airbyte/container_orchestrator/DbtJobOrchestrator.java

...orchestrator/src/main/java/io/airbyte/container_orchestrator/ReplicationJobOrchestrator.java

airbyte-workers/src/main/java/io/airbyte/workers/process/KubePodProcess.java

davinchia

Looks great! Much much cleaner, especially the state management, which is mostly contained in 1/2 classes now.

Some questions for clarity. Can I confirm, the orchestrator stuff won't be available for docker yeah?

Feel free to merge once you have dealt with the comments.

airbyte-config/models/src/main/java/io/airbyte/config/storage/DefaultS3ClientFactory.java

...ontainer-orchestrator/src/main/java/io/airbyte/container_orchestrator/AsyncStateManager.java

...r-orchestrator/src/main/java/io/airbyte/container_orchestrator/DefaultAsyncStateManager.java

...r-orchestrator/src/main/java/io/airbyte/container_orchestrator/ContainerOrchestratorApp.java

airbyte-workers/src/main/java/io/airbyte/workers/process/AsyncOrchestratorPodProcess.java

airbyte-workers/src/main/java/io/airbyte/workers/process/KubePodProcess.java

airbyte-workers/src/main/java/io/airbyte/workers/temporal/sync/LauncherWorker.java

davinchia · 2022-01-14T10:58:15Z

kube/resources/worker.yaml

@@ -184,6 +184,32 @@ spec:
                configMapKeyRef:
                  name: airbyte-env
                  key: JOB_KUBE_MAIN_CONTAINER_IMAGE_PULL_POLICY
+            # todo: add other state storage keys
+            - name: STATE_STORAGE_MINIO_BUCKET_NAME


we can add in a follow up PR - should also update the https://docs.airbyte.com/operator-guides/configuring-airbyte#kubernetes-only with the state env vars.

…ch-via-cloud-storage

jrhizor added 3 commits December 24, 2021 15:27

add misc todos

cb879c5

save work so far

a1e320e

configure async pod processing

017c27f

github-actions bot added area/platform issues related to the platform area/worker Related to worker labels Jan 1, 2022

jrhizor commented Jan 1, 2022

View reviewed changes

jrhizor temporarily deployed to more-secrets January 1, 2022 01:45 Inactive

jrhizor added 2 commits December 31, 2021 17:45

remove comment

31c129d

fmt

2573d97

jrhizor temporarily deployed to more-secrets January 1, 2022 01:47 Inactive

jrhizor added 2 commits January 3, 2022 09:30

Merge branch 'master' into jrhizor/async-kube-pod-process-with-reatta…

c62ce5b

…ch-via-cloud-storage

Merge branch 'master' into jrhizor/async-kube-pod-process-with-reatta…

a24bfbd

…ch-via-cloud-storage

jrhizor temporarily deployed to more-secrets January 3, 2022 17:33 Inactive

jrhizor added 2 commits January 6, 2022 06:35

Merge branch 'master' into jrhizor/async-kube-pod-process-with-reatta…

ca7b9dd

…ch-via-cloud-storage

working except logging propagation?

5eaf0d3

jrhizor temporarily deployed to more-secrets January 6, 2022 15:56 Inactive

jrhizor requested review from davinchia and benmoriceau January 6, 2022 16:02

benmoriceau reviewed Jan 6, 2022

View reviewed changes

add comment

34a4f69

jrhizor added 2 commits January 9, 2022 19:09

Merge branch 'master' into jrhizor/async-kube-pod-process-with-reatta…

608189a

…ch-via-cloud-storage

add logging and misc configuration fixes

e1677b1

github-actions bot added the kubernetes label Jan 10, 2022

jrhizor temporarily deployed to more-secrets January 10, 2022 07:35 Inactive

add output propagation

e8a1b1d

jrhizor temporarily deployed to more-secrets January 10, 2022 17:51 Inactive

Merge branch 'master' into jrhizor/async-kube-pod-process-with-reatta…

4858315

…ch-via-cloud-storage

jrhizor temporarily deployed to more-secrets January 10, 2022 18:29 Inactive

jrhizor temporarily deployed to more-secrets January 10, 2022 23:00 Inactive

comment

7564a9b

jrhizor temporarily deployed to more-secrets January 11, 2022 07:07 Inactive

clean up and test state management

20739d3

jrhizor temporarily deployed to more-secrets January 12, 2022 21:49 Inactive

clean up orchestrator app construction

295fd09

jrhizor temporarily deployed to more-secrets January 12, 2022 22:18 Inactive

unify launcher workers and handle resuming

c6a53ca

jrhizor temporarily deployed to more-secrets January 13, 2022 15:18 Inactive

respond to comments

318659a

jrhizor requested a review from benmoriceau January 13, 2022 15:21

jrhizor marked this pull request as ready for review January 13, 2022 15:21

Merge branch 'master' into jrhizor/async-kube-pod-process-with-reatta…

99f24b6

…ch-via-cloud-storage

jrhizor temporarily deployed to more-secrets January 13, 2022 15:23 Inactive

misc

32ce6e5

jrhizor temporarily deployed to more-secrets January 13, 2022 15:26 Inactive

disable

a9122d1

jrhizor temporarily deployed to more-secrets January 13, 2022 15:31 Inactive

fix comment

21d7064

jrhizor temporarily deployed to more-secrets January 13, 2022 15:45 Inactive

benmoriceau approved these changes Jan 14, 2022

View reviewed changes

davinchia approved these changes Jan 14, 2022

View reviewed changes

respond to comments

49fe601

jrhizor temporarily deployed to more-secrets January 20, 2022 15:53 Inactive

Merge branch 'master' into jrhizor/async-kube-pod-process-with-reatta…

5a49325

…ch-via-cloud-storage

jrhizor merged commit db40932 into master Jan 20, 2022

jrhizor deleted the jrhizor/async-kube-pod-process-with-reattach-via-cloud-storage branch January 20, 2022 15:56

jrhizor temporarily deployed to more-secrets January 20, 2022 15:56 Inactive

octavia-squidington-iii mentioned this pull request Jan 21, 2022

Bump Airbyte version from 0.35.6-alpha to 0.35.7-alpha #9708

Merged

jrhizor mentioned this pull request Jan 24, 2022

use path for GOOGLE_APPLICATION_CREDENTIALS in client config #9763

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

async container launch kubernetes "process" #9242

async container launch kubernetes "process" #9242

jrhizor commented Jan 1, 2022 •

edited

Loading

jrhizor Jan 1, 2022

jrhizor commented Jan 6, 2022

benmoriceau Jan 6, 2022

jrhizor Jan 13, 2022

davinchia Jan 14, 2022

benmoriceau Jan 6, 2022

jrhizor Jan 13, 2022

davinchia Jan 14, 2022

davinchia commented Jan 7, 2022 •

edited

Loading

davinchia left a comment

davinchia Jan 14, 2022

async container launch kubernetes "process" #9242

async container launch kubernetes "process" #9242

Conversation

jrhizor commented Jan 1, 2022 • edited Loading

Choose a reason for hiding this comment

jrhizor commented Jan 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davinchia commented Jan 7, 2022 • edited Loading

davinchia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrhizor commented Jan 1, 2022 •

edited

Loading

davinchia commented Jan 7, 2022 •

edited

Loading