fix terminating thread kube #3610

jrhizor · 2021-05-26T02:00:45Z

Definitely not mergable, but I wanted to demonstrate one way we can do this.

HttpClientUtilsAirbyte is a clone of io.fabric8.kubernetes.client.utils.HttpClientUtils except with the inclusion of the following line:

httpClientBuilder.connectionPool(new ConnectionPool(0, 30, TimeUnit.SECONDS));

Since it specifies that we can't keep idle connections around it will incur a small performance hit on the kube api server, but it would fix this problem for now at least. We could also allow idle connections but move the keep alive down to 1s/500ms/similar.

I tried just using

OkHttpClient.Builder httpClientBuilder = new OkHttpClient.Builder();
httpClientBuilder.connectionPool(new ConnectionPool(0, 30, TimeUnit.SECONDS));
httpClientBuilder.build();

but even in Docker for Desktop Kube it fails because it doesn't have all of that SSL configuration.

I bet there's an OkHttp client builder somewhere that is serviceable by default.

…umber of records and always works.

… pod.

…It also waits for the pod to be ready before doing anything else. Sync worker will also remove the pod on termination.

…rbyte into davinchia/kube-queueing-poc

…elete command in Sync worker.

…rbyte into davinchia/kube-queueing-poc

* nearly working sources * update * stdin example

…t the airbyte-workers resource folder; place all the poc yamls together.

…es to Dest pod after refactor.

…ot seem to be passing in.

…rbyte into davinchia/kube-queueing-poc

also clean up kubernetes deploys.

…getting the right entrypoint.

jrhizor · 2021-05-26T02:01:06Z

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

@@ -95,9 +99,10 @@ public static void main(String[] args) throws InterruptedException, IOException
    // https://square.github.io/okhttp/4.x/okhttp/okhttp3/-ok-http-client/#shutdown-isnt-necessary
    // Instead, the pod shuts down after 5 minutes as the pool reaps the remaining idle connection after
    // 5 minutes of inactivity, as per the default configuration.
+    KUBE_CLIENT.close();
+    OK_HTTP_CLIENT.dispatcher().cancelAll();


this had no impact

jrhizor · 2021-05-26T02:01:16Z

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

@@ -95,9 +99,10 @@ public static void main(String[] args) throws InterruptedException, IOException
    // https://square.github.io/okhttp/4.x/okhttp/okhttp3/-ok-http-client/#shutdown-isnt-necessary
    // Instead, the pod shuts down after 5 minutes as the pool reaps the remaining idle connection after
    // 5 minutes of inactivity, as per the default configuration.
+    KUBE_CLIENT.close();


reordering had no impact

jrhizor · 2021-05-26T02:01:24Z

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

+    // Explicitly create the underlying HTTP client since the Kube client has issues with closing the
+    // client. It is not clear in which library the fault
+    // lies. See https://github.com/fabric8io/kubernetes-client/issues/2403.
+    final Config CONFIG = new ConfigBuilder().build();


making it non-static had no impact

jrhizor · 2021-05-26T02:01:36Z

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

+    // client. It is not clear in which library the fault
+    // lies. See https://github.com/fabric8io/kubernetes-client/issues/2403.
+    final Config CONFIG = new ConfigBuilder().build();
+    final OkHttpClient OK_HTTP_CLIENT = HttpClientUtilsAirbyte.createHttpClient(CONFIG);


this is the only piece that had impact

davinchia · 2021-05-26T02:04:10Z

nice! was thinking this is the 'best' way to do so.

I'm thinking we use system.exit for now, and open a PR against fabric to make their constructor configurable so we have less code to maintain. wdyt?

jrhizor · 2021-05-26T02:11:37Z

I'd actually prefer to clean this up slightly merge this in with a big comment linking to an issue and a fabric8 change. It looks like if it were accepted it'd take up to a month to get in place. We're going to need proper termination in a week.

Either that or I think we'd need to wrap everything in our main loop with a try catch that finally calls system.exit?

Either way I'll make the PR for fabric8 in the morning in hopes to get that into their next release ASAP.

davinchia · 2021-05-26T02:52:05Z

Makes sense. I'll create an issue for us and Fabric and use the system.exit version in the mean time.

I'm going to start using the cloud tag.

davinchia and others added 30 commits May 18, 2021 11:30

Use CDK to generate source that can be configured to emit a certain n…

a934b68

…umber of records and always works.

Checkpoint: socat works from inside the docker container.

d2ab458

Override the entry point.

1ee5c5a

Clean up and add ReadMe.

001b4a7

Clean up socat.

b90ec71

Merge remote-tracking branch 'origin' into davinchia/kube-queueing-poc

4386d49

Checkpoint: connect to Kube cluster and list all the pods.

1fb1b84

Checkpoint: Sync worker pod is able to send output to the destination…

bd3f17f

… pod.

Checkpoint: Sync worker creates Dest pod if none existed previously. …

f9a6c9b

…It also waits for the pod to be ready before doing anything else. Sync worker will also remove the pod on termination.

update readme

921af61

Merge branch 'davinchia/kube-queueing-poc' of github.com:airbytehq/ai…

de79c79

…rbyte into davinchia/kube-queueing-poc

Checkpoint: Dest pod does nott restart after finishing. Comment out d…

c01a386

…elete command in Sync worker.

Merge branch 'davinchia/kube-queueing-poc' of github.com:airbytehq/ai…

5a53726

…rbyte into davinchia/kube-queueing-poc

working towards named pipes

435e9db

Merge branch 'davinchia/kube-queueing-poc' of github.com:airbytehq/ai…

39707d4

…rbyte into davinchia/kube-queueing-poc

named pipes working

b813830

update readme

6f13510

WIP named pipe / socat sidecar kube port forwarding (#3518)

0a59f84

* nearly working sources * update * stdin example

move all kube testing yamls into the airbyte-workers directories. sor…

c4d976b

…t the airbyte-workers resource folder; place all the poc yamls together.

Format.

16fca70

Put back the original KubeProcessBuilderFactory.

01be8ef

Fix slight errors.

e8849f7

Checkpoint: Worker pod knows its own IP. Successfully starts and writ…

2cdb8f5

…es to Dest pod after refactor.

remove unused file and update readme

a9bb131

Dest pod loops back into worker pod. However, the right messages do n…

fc19aff

…ot seem to be passing in.

Switch back to worker ip.

6180099

Merge branch 'davinchia/kube-queueing-poc' of github.com:airbytehq/ai…

4a3666c

…rbyte into davinchia/kube-queueing-poc

SWEET VICTORY!.

4109ef7

wrap kube pod in process (#3540)

b447fe8

also clean up kubernetes deploys.

Checkpoint: create getCommandFromImage test.

5517a10

davinchia and others added 9 commits May 25, 2021 17:06

Update test containers.

fb41ce1

Use fabric api for getting entrypoint. Add tests to make sure we are …

6dd0c4a

…getting the right entrypoint.

Remove kubectl install dependency.

cfed967

Clean up getPodId and add tests.

582df9d

Make sure all client.pod calls include namespace.

9aae5da

Add comments to explain what is happening.

2072dd0

Format.

a24afdb

Remove unused test dockerfile.

193f994

prevent kube pod process from hanging

f0a53c0

jrhizor requested a review from davinchia May 26, 2021 02:00

auto-assign bot requested review from cgardens and masonwheeler May 26, 2021 02:00

jrhizor removed request for masonwheeler and cgardens May 26, 2021 02:00

jrhizor commented May 26, 2021

View reviewed changes

Base automatically changed from davinchia/kube-api-cleanup to davinchia/kube-queueing-poc May 26, 2021 04:30

Base automatically changed from davinchia/kube-queueing-poc to master June 10, 2021 01:12

jrhizor closed this Jul 13, 2021

jrhizor deleted the jrhizor/fix-terminating-thread-kube branch July 13, 2021 23:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix terminating thread kube #3610

fix terminating thread kube #3610

jrhizor commented May 26, 2021

jrhizor May 26, 2021

jrhizor May 26, 2021

jrhizor May 26, 2021

jrhizor May 26, 2021

davinchia commented May 26, 2021

jrhizor commented May 26, 2021

davinchia commented May 26, 2021 •

edited

Loading

fix terminating thread kube #3610

fix terminating thread kube #3610

Conversation

jrhizor commented May 26, 2021

jrhizor May 26, 2021

Choose a reason for hiding this comment

jrhizor May 26, 2021

Choose a reason for hiding this comment

jrhizor May 26, 2021

Choose a reason for hiding this comment

jrhizor May 26, 2021

Choose a reason for hiding this comment

davinchia commented May 26, 2021

jrhizor commented May 26, 2021

davinchia commented May 26, 2021 • edited Loading

davinchia commented May 26, 2021 •

edited

Loading