feat: Network transport (2) - TCP/KCP protocol can be switched by cfg #1895

michaljurecko · 2024-07-09T11:43:10Z

Jira: https://keboola.atlassian.net/browse/PSGO-597

See #1894

Changes:

TCP/KCP protocol can be switched by configuration.
- Default value is TCP.
- It will be good enough for start and save time.
- UPD Buffer Size is necessary to achieve high performance, but it is not easy to set up in Kubernetes (I don't know which defaults value are in our stacks).
Improved tests.
Related to:
- feat: Add disk writer network transport layer (kcp-go/yamux) #1894 (comment)
- feat: Add disk writer network transport layer (kcp-go/yamux) #1894 (comment)

KCP is on localhost 2-3x slower than TCP, but it is expected.

TCP runs in kernel space and has various optimalization for localhost.
KCP runs in user space and its benefits can be seen on real networks (see projects like: kcptun, frp, syncthing, ...)

dataSize = 100MB - both protocols are fast enough up to 500MB/s on localhost.

Read more (quic is another R-UDP protocol, the principle is the same):

michaljurecko · 2024-07-09T12:13:34Z

.github/workflows/test-lint.yml


+      - name: Free Disk Space (Ubuntu)
+        uses: jlumbroso/free-disk-space@main
+        with:
+          android: false
+          dotnet: true
+          haskell: false
+          large-packages: false
+          docker-images: false
+          swap-storage: false
+
      - name: Setup Go, tools and caching


(maybe it needs some better fix, It looks like, the linter cache is growing and growing)

@michaljurecko We are encoutering this issue in other PRs. Should we disable caching if the linter cache is growing? Currently I do not have much space to investigate it.

~~@jachym-tousek-keboola Petr needs the fix, I am mergin it, you comment will be addresed in next PRs, if any.~~

internal/pkg/service/stream/storage/level/local/diskwriter/network/transport/transport_kcp.go

internal/pkg/service/stream/storage/level/local/diskwriter/network/transport/util.go

internal/pkg/service/stream/storage/level/local/diskwriter/network/transport/transport_kcp.go

michaljurecko · 2024-07-10T06:28:26Z

internal/pkg/service/stream/storage/level/local/diskwriter/network/transport/transport_kcp.go

+	c.SetACKNoDelay(true) // send data immediately, needed for bigger payloads
+	c.SetNoDelay(1, 10, 2, 1)


For context, why?

During testing of upper layers, I found that writing 100MB takes 5s to write.

It didn't fit in any way with the previous experiments that I did when I was choosing the protocol.

That's why I added the TCP protocol there, for comparison.

Finally, after a few hours, I realized that I forgot to set SetACKNoDelay(true), which makes the difference.

Otherwise, it waits for the next sending window, triggered by a timer.

It makes sense for small messages that are smaller than an UDP packet (~1KB), but for our 10KB+ records it slows down the write (we have buffers in the pipeline).

github-actions · 2024-07-10T06:42:18Z

Stream Kubernetes Diff [CI]

Between base 679b6e7 ⬅️ head 582ec36.

Expand

--- /tmp/artifacts/test-k8s-state.old.json.processed.kv	2024-07-10 06:42:14.169618260 +0000
+++ /tmp/artifacts/test-k8s-state.new.json.processed.kv	2024-07-10 06:42:14.617623054 +0000
@@ -13 +13 @@
-<ConfigMap/stream-config>.data["config.yaml"] = "###############################################################################################...
+<ConfigMap/stream-config>.data["config.yaml"] = "###############################################################################################...
@@ -194 +194 @@
-<Deployment/stream-api>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<Deployment/stream-api>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -352 +352 @@
-<Deployment/stream-http-source>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<Deployment/stream-http-source>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -507 +507 @@
-<Deployment/stream-storage-coordinator>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<Deployment/stream-storage-coordinator>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -1186,2 +1186,2 @@
-<Pod/stream-api-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
-<Pod/stream-api-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<Pod/stream-api-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
+<Pod/stream-api-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -1506 +1506 @@
-<Pod/stream-etcd-0>.spec.containers[0].env[21].value = "new";
+<Pod/stream-etcd-0>.spec.containers[0].env[21].value = "existing";
@@ -1748 +1748 @@
-<Pod/stream-etcd-1>.spec.containers[0].env[21].value = "new";
+<Pod/stream-etcd-1>.spec.containers[0].env[21].value = "existing";
@@ -2056,2 +2056,2 @@
-<Pod/stream-http-source-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
-<Pod/stream-http-source-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<Pod/stream-http-source-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
+<Pod/stream-http-source-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -2436,2 +2436,2 @@
-<Pod/stream-storage-coordinator-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
-<Pod/stream-storage-coordinator-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<Pod/stream-storage-coordinator-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
+<Pod/stream-storage-coordinator-<hash>>.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -2663 +2663 @@
-<Pod/stream-storage-writer-reader-0>.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<Pod/stream-storage-writer-reader-0>.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -2739 +2739 @@
-<Pod/stream-storage-writer-reader-0>.spec.containers[1].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<Pod/stream-storage-writer-reader-0>.spec.containers[1].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -2910 +2910 @@
-<Pod/stream-storage-writer-reader-1>.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<Pod/stream-storage-writer-reader-1>.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -2986 +2986 @@
-<Pod/stream-storage-writer-reader-1>.spec.containers[1].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<Pod/stream-storage-writer-reader-1>.spec.containers[1].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -3038 +3038 @@
-<Pod/stream-storage-writer-reader-1>.spec.volumes[0].persistentVolumeClaim.claimName = "medium-001-stream-storage-writer-reader-1";
+<Pod/stream-storage-writer-reader-1>.spec.volumes[0].persistentVolumeClaim.claimName = "fast-001-stream-storage-writer-reader-1";
@@ -3041 +3041 @@
-<Pod/stream-storage-writer-reader-1>.spec.volumes[1].persistentVolumeClaim.claimName = "slow-001-stream-storage-writer-reader-1";
+<Pod/stream-storage-writer-reader-1>.spec.volumes[1].persistentVolumeClaim.claimName = "medium-001-stream-storage-writer-reader-1";
@@ -3044 +3044 @@
-<Pod/stream-storage-writer-reader-1>.spec.volumes[2].persistentVolumeClaim.claimName = "fast-001-stream-storage-writer-reader-1";
+<Pod/stream-storage-writer-reader-1>.spec.volumes[2].persistentVolumeClaim.claimName = "slow-001-stream-storage-writer-reader-1";
@@ -3228 +3228 @@
-<ReplicaSet/stream-api-<hash>>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<ReplicaSet/stream-api-<hash>>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -3393 +3393 @@
-<ReplicaSet/stream-http-source-<hash>>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<ReplicaSet/stream-http-source-<hash>>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -3555 +3555 @@
-<ReplicaSet/stream-storage-coordinator-<hash>>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<ReplicaSet/stream-storage-coordinator-<hash>>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -3596,0 +3597,12 @@
+<Secret/sh.helm.release.v1.stream-etcd.v2> = {};
+<Secret/sh.helm.release.v1.stream-etcd.v2>.apiVersion = "v1";
+<Secret/sh.helm.release.v1.stream-etcd.v2>.data = {};
+<Secret/sh.helm.release.v1.stream-etcd.v2>.kind = "Secret";
+<Secret/sh.helm.release.v1.stream-etcd.v2>.metadata = {};
+<Secret/sh.helm.release.v1.stream-etcd.v2>.metadata.labels = {};
+<Secret/sh.helm.release.v1.stream-etcd.v2>.metadata.labels.name = "stream-etcd";
+<Secret/sh.helm.release.v1.stream-etcd.v2>.metadata.labels.owner = "helm";
+<Secret/sh.helm.release.v1.stream-etcd.v2>.metadata.labels.version = "2";
+<Secret/sh.helm.release.v1.stream-etcd.v2>.metadata.name = "sh.helm.release.v1.stream-etcd.v2";
+<Secret/sh.helm.release.v1.stream-etcd.v2>.metadata.namespace = "stream";
+<Secret/sh.helm.release.v1.stream-etcd.v2>.type = "helm.sh/release.v1";
@@ -3862 +3874 @@
-<StatefulSet/stream-etcd>.spec.template.spec.containers[0].env[21].value = "new";
+<StatefulSet/stream-etcd>.spec.template.spec.containers[0].env[21].value = "existing";
@@ -4099 +4111 @@
-<StatefulSet/stream-storage-writer-reader>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<StatefulSet/stream-storage-writer-reader>.spec.template.spec.containers[0].image = "docker.io/keboola/stream-api:582ec36-1720593493";
@@ -4172 +4184 @@
-<StatefulSet/stream-storage-writer-reader>.spec.template.spec.containers[1].image = "docker.io/keboola/stream-api:95f6057-1720593118";
+<StatefulSet/stream-storage-writer-reader>.spec.template.spec.containers[1].image = "docker.io/keboola/stream-api:582ec36-1720593493";


(see artifacts in the Github Action for more information)

Matovidlo

LGTM

jachym-tousek-keboola

Looks good but how did you compare TCP and KCP? Did you write some benchmarks?

michaljurecko · 2024-07-10T08:58:46Z

Looks good but how did you compare TCP and KCP? Did you write some benchmarks?

There is test, which logs write duration, see code.

If you want, write a benchmark, but as I wrote, it will not be sufficiently descriptive on localhost.

I only needed such a simple comparison to find the issue, so the test was enough.

michaljurecko changed the title ~~feat: Network transport ()~~ feat: Network transport (2) Jul 9, 2024

michaljurecko force-pushed the michaljurecko-PSGO-591-network-transport-2 branch 2 times, most recently from e133727 to a2089b8 Compare July 9, 2024 12:05

michaljurecko added streams-http-source streams-R1 labels Jul 9, 2024

michaljurecko commented Jul 9, 2024

View reviewed changes

michaljurecko force-pushed the michaljurecko-PSGO-591-network-transport-2 branch 4 times, most recently from 807d4f0 to 2acaa38 Compare July 9, 2024 13:00

michaljurecko changed the title ~~feat: Network transport (2)~~ feat: Network transport (2) - TCP/KCP protocol can be switched by cfg Jul 9, 2024

michaljurecko added 2 commits July 9, 2024 15:12

feat: Improve network config

d49f18c

feat: Make network transport protocol configurable TCP/KCP

8f8cfae

michaljurecko force-pushed the michaljurecko-PSGO-591-network-transport-2 branch from 2acaa38 to df4487b Compare July 9, 2024 13:12

michaljurecko added 2 commits July 9, 2024 15:25

tests: Improve tests

f1e4ab3

build: Fix mkdir /tmp/....: no space left on device

323d96d

michaljurecko force-pushed the michaljurecko-PSGO-591-network-transport-2 branch from df4487b to 323d96d Compare July 9, 2024 13:25

michaljurecko mentioned this pull request Jul 9, 2024

feat: Add disk writer network transport layer (kcp-go/yamux) #1894

Merged

michaljurecko marked this pull request as ready for review July 9, 2024 13:39

michaljurecko requested review from jachym-tousek-keboola and Matovidlo July 10, 2024 04:29

Matovidlo reviewed Jul 10, 2024

View reviewed changes

internal/pkg/service/stream/storage/level/local/diskwriter/network/transport/transport_kcp.go Show resolved Hide resolved

internal/pkg/service/stream/storage/level/local/diskwriter/network/transport/util.go Show resolved Hide resolved

michaljurecko commented Jul 10, 2024

View reviewed changes

docs: Add comment - disabled logger

582ec36

michaljurecko requested a review from Matovidlo July 10, 2024 06:31

Matovidlo approved these changes Jul 10, 2024

View reviewed changes

jachym-tousek-keboola approved these changes Jul 10, 2024

View reviewed changes

michaljurecko merged commit a9a876c into main Jul 10, 2024
14 checks passed

michaljurecko deleted the michaljurecko-PSGO-591-network-transport-2 branch July 10, 2024 08:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Network transport (2) - TCP/KCP protocol can be switched by cfg #1895

feat: Network transport (2) - TCP/KCP protocol can be switched by cfg #1895

michaljurecko commented Jul 9, 2024 •

edited

Loading

michaljurecko Jul 9, 2024

michaljurecko Jul 10, 2024

Matovidlo Jul 10, 2024

michaljurecko Jul 10, 2024 •

edited

Loading

michaljurecko Jul 10, 2024 •

edited

Loading

github-actions bot commented Jul 10, 2024

Matovidlo left a comment

jachym-tousek-keboola left a comment

michaljurecko commented Jul 10, 2024

		c.SetACKNoDelay(true) // send data immediately, needed for bigger payloads
		c.SetNoDelay(1, 10, 2, 1)

feat: Network transport (2) - TCP/KCP protocol can be switched by cfg #1895

feat: Network transport (2) - TCP/KCP protocol can be switched by cfg #1895

Conversation

michaljurecko commented Jul 9, 2024 • edited Loading

michaljurecko Jul 9, 2024

Choose a reason for hiding this comment

michaljurecko Jul 10, 2024

Choose a reason for hiding this comment

Matovidlo Jul 10, 2024

Choose a reason for hiding this comment

michaljurecko Jul 10, 2024 • edited Loading

Choose a reason for hiding this comment

michaljurecko Jul 10, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jul 10, 2024

Stream Kubernetes Diff [CI]

Matovidlo left a comment

Choose a reason for hiding this comment

jachym-tousek-keboola left a comment

Choose a reason for hiding this comment

michaljurecko commented Jul 10, 2024

michaljurecko commented Jul 9, 2024 •

edited

Loading

michaljurecko Jul 10, 2024 •

edited

Loading

michaljurecko Jul 10, 2024 •

edited

Loading