maintain cluster active connection counter for circuit breaker #570

Okabe-Rintarou-0 · 2024-07-12T14:11:05Z

What type of PR is this?
feature

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Proposal here #397

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Okabe-Rintarou-0 · 2024-07-12T14:12:05Z

how should i get the cluster id? Using cluster name will exceed the stack size limit？
may be extract hashName to a public package?

Okabe-Rintarou-0 · 2024-07-13T01:55:33Z

It's quite confusing

bpf/kmesh/ads/include/circuit_breaker.h

LiZhenCheng9527 · 2024-07-13T03:43:31Z

Some files need to be updated, please run 'make gen' and include any changed files in your PR

codecov · 2024-07-13T06:44:55Z

Codecov Report

Attention: Patch coverage is 64.06250% with 23 lines in your changes missing coverage. Please review.

Project coverage is 49.54%. Comparing base (1902392) to head (e9af645).
Report is 48 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/cache/v2/cluster.go	79.41%	5 Missing and 2 partials ⚠️
pkg/bpf/bpf.go	0.00%	5 Missing ⚠️
pkg/utils/hash_name.go	0.00%	4 Missing ⚠️
pkg/controller/ads/cache.go	66.66%	3 Missing ⚠️
pkg/controller/controller.go	0.00%	3 Missing ⚠️
pkg/controller/workload/workload_processor.go	50.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
pkg/bpf/ads/loader.go	`47.43% <100.00%> (+1.38%)`	⬆️
pkg/bpf/workload/sock_connection.go	`54.01% <ø> (ø)`
pkg/bpf/workload/sock_ops.go	`55.88% <ø> (ø)`
pkg/bpf/workload/xdp.go	`48.57% <ø> (ø)`
pkg/controller/ads/ads_controller.go	`80.70% <100.00%> (ø)`
pkg/controller/ads/ads_processor.go	`75.33% <100.00%> (ø)`
pkg/controller/client.go	`65.62% <100.00%> (ø)`
pkg/controller/workload/workload_processor.go	`61.67% <50.00%> (+2.51%)`	⬆️
pkg/controller/ads/cache.go	`48.67% <66.66%> (+0.16%)`	⬆️
pkg/controller/controller.go	`0.00% <0.00%> (ø)`
... and 3 more

... and 10 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4ca182...e9af645. Read the comment docs.

Okabe-Rintarou-0 · 2024-07-16T03:00:14Z

/unhold

Okabe-Rintarou-0 · 2024-07-21T02:08:47Z

I use a special way to reject connection, plz check MARK_REJECTED macro.

Tested in oe23.03.

hzxuzhonghu

mostly LGTM

Defer to @nlgwcy for the connection reset

hzxuzhonghu · 2024-07-23T02:32:36Z

api/cluster/cluster.proto


  core.ApiStatus api_status = 128;
  string name = 1;
+  uint32 id = 2;


weired, seems the tag number is random

hzxuzhonghu · 2024-07-23T02:39:38Z

bpf/kmesh/ads/include/circuit_breaker.h

+    stats = kmesh_map_lookup_elem(&map_of_cluster_stats, key);
+    if (!stats) {
+        struct cluster_stats new_stats = {0};
+        new_stats.active_connections = delta;


would it happen delta = -1 here

yes, need a check here.

bpf/kmesh/ads/include/circuit_breaker.h

hzxuzhonghu · 2024-07-23T02:43:54Z

bpf/kmesh/ads/include/circuit_breaker.h

+                "Current active connections %d exceeded max connections %d, reject connection\n",
+                stats->active_connections,
+                cbs->max_connections);
+            return 0;


We should always use 0 as success and negative number as error code

hzxuzhonghu · 2024-07-23T02:47:51Z

bpf/kmesh/ads/include/ctx/sock_addr.h

    (ctx)->user_ip4 = (address)->ipv4;                                                                                 \
    (ctx)->user_port = (address)->port

+#define MARK_REJECTED(ctx)


not sure what is this for

cgroups/connect4 and sockops both use cluster_manager, without it will compile fail

In kernel-native mode, if the running environment is not enhanced kernel(that is, L7 kernel traffic management is not supported), Kmesh completes L4 traffic management in the connect hook.

// cgroup_sock.c cgroup_connect4_prog -> sock4_traffic_control -> listener_manager -> SEC_TAIL(KMESH_TAIL_CALL_FILTER_CHAIN) -> SEC_TAIL(KMESH_TAIL_CALL_FILTER) -> tcp_proxy_manager -> SEC_TAIL(KMESH_TAIL_CALL_CLUSTER) -> on_cluster_sock_bind

So, on this path, if circuit breaker is triggered, the service also needs to be rejected.

hzxuzhonghu · 2024-07-23T02:50:07Z

bpf/kmesh/ads/sockops.c

+        }
+        break;
+    case BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB:
+        if (bpf_sock_ops_cb_flags_set(skops, BPF_SOCK_OPS_STATE_CB_FLAG) != 0) {


why set cb for inbound

I will delete it

hzxuzhonghu · 2024-07-23T02:52:50Z

kernel/ko_src/kmesh/defer_connect.c

+    if (daddr == 0 && dport == 0) {
+        tcp_set_state(sk, TCP_CLOSE);
+        sk->sk_route_caps = 0;
+        inet_sk(sk)->inet_dport = 0;
+        err = -1;
+        goto out;


@nlgwcy to review

hzxuzhonghu · 2024-07-23T06:34:11Z

bpf/kmesh/ads/include/circuit_breaker.h

                stats->active_connections,
                cbs->max_connections);
-            return 0;
+            return 1;


Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

Signed-off-by: 923048992@qq.com <923048992@qq.com>

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

Signed-off-by: 923048992@qq.com <923048992@qq.com>

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

Signed-off-by: 923048992@qq.com <923048992@qq.com>

bpf/kmesh/ads/include/circuit_breaker.h

hzxuzhonghu · 2024-10-28T03:01:37Z

pkg/bpf/bpf_kmesh.go

+ * limitations under the License.
+
+ * Author: nlgwcy
+ * Create: 2022-02-26


hzxuzhonghu · 2024-10-28T03:02:36Z

pkg/bpf/bpf_kmesh.go

+	"kmesh.net/kmesh/daemon/options"
+)
+
+type BpfTracePoint struct {


We have split bpf progs, should not revert

hzxuzhonghu · 2024-10-28T03:08:01Z

pkg/cache/v2/cluster.go

 	// resourceHash[0]:cds  resourceHash[1]:eds
-	resourceHash map[string][2]uint64
+	resourceHash    map[string][2]uint64
+	hashName        *utils.HashName


There is persisting in hashName, do we need it here?

If not, i would think we can pkg github.com/google/uuid to generate uuid

hzxuzhonghu · 2024-10-28T03:15:16Z

pkg/cache/v2/cluster.go

+
+	for it.Next(&key, &value) {
+		if key.ClusterId == clusterId {
+			log.Debugf("remove cluster stats with key %v", key)


nit: remove

Signed-off-by: 923048992@qq.com <923048992@qq.com>

hzxuzhonghu

#570 (comment) can be done later

/lgtm

kmesh-bot · 2024-10-30T09:59:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [hzxuzhonghu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kmesh-bot added the do-not-merge/work-in-progress label Jul 12, 2024

kmesh-bot requested review from hzxuzhonghu and nlgwcy July 12, 2024 14:11

kmesh-bot added the size/L label Jul 12, 2024

LiZhenCheng9527 reviewed Jul 13, 2024

View reviewed changes

bpf/kmesh/ads/include/circuit_breaker.h Show resolved Hide resolved

Okabe-Rintarou-0 force-pushed the feat/cluster_conn branch from 3fc8d0f to 53ab706 Compare July 13, 2024 06:36

kmesh-bot added the do-not-merge/hold label Jul 13, 2024

Okabe-Rintarou-0 mentioned this pull request Jul 15, 2024

What about extract hashName to a separate package #578

Closed

kmesh-bot added size/XL and removed size/L labels Jul 16, 2024

kmesh-bot added ok-to-test size/XXL and removed do-not-merge/hold size/XL labels Jul 16, 2024

Okabe-Rintarou-0 force-pushed the feat/cluster_conn branch from e4a76b0 to 86004b1 Compare July 16, 2024 12:32

kmesh-bot added size/XL and removed size/XXL labels Jul 16, 2024

Okabe-Rintarou-0 force-pushed the feat/cluster_conn branch 2 times, most recently from 0119ade to a9e3c58 Compare July 16, 2024 14:06

Okabe-Rintarou-0 changed the title ~~[WIP] maintain cluster active connection counter for circuit breaker~~ maintain cluster active connection counter for circuit breaker Jul 21, 2024

kmesh-bot removed the do-not-merge/work-in-progress label Jul 21, 2024

Okabe-Rintarou-0 force-pushed the feat/cluster_conn branch from 9138cef to 1eb8103 Compare July 23, 2024 01:48

hzxuzhonghu reviewed Jul 23, 2024

View reviewed changes

bpf/kmesh/ads/include/circuit_breaker.h Outdated

stats->active_connections,

cbs->max_connections);

return 0;

return 1;

Copy link

Member

hzxuzhonghu Jul 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1

Okabe-Rintarou-0 added 13 commits October 24, 2024 20:36

remove cluster_sock_connect hook in passive connect cb

e9d88a6

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

fix logic

a2f7b49

Signed-off-by: 923048992@qq.com <923048992@qq.com>

add connection reject logic

1125f80

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

fix lint

09a0dcc

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

fix lint

2427a1d

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

fix some errors

5eb14e0

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

make it thread safe

a8ff81f

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

change return val

5d476ab

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

add log type

e962280

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

clear cluster stats in user space

573156f

Signed-off-by: 923048992@qq.com <923048992@qq.com>

fix delete logic

9a3299d

Signed-off-by: Okabe-Rintarou-0 <923048992@qq.com>

fix lint

283ad72

Signed-off-by: 923048992@qq.com <923048992@qq.com>

add ut

8615e75

Signed-off-by: 923048992@qq.com <923048992@qq.com>

Okabe-Rintarou-0 force-pushed the feat/cluster_conn branch from abfa7ab to 8615e75 Compare October 24, 2024 13:43

kmesh-bot added size/XXL and removed size/XL labels Oct 24, 2024

Okabe-Rintarou-0 force-pushed the feat/cluster_conn branch 2 times, most recently from b9585ec to 2510d8f Compare October 24, 2024 14:25

hzxuzhonghu reviewed Oct 28, 2024

View reviewed changes

kmesh-bot added size/XL and removed size/XXL labels Oct 28, 2024

Okabe-Rintarou-0 force-pushed the feat/cluster_conn branch from 5345d93 to 0941b43 Compare October 28, 2024 05:36

optimize logic

e9af645

Signed-off-by: 923048992@qq.com <923048992@qq.com>

Okabe-Rintarou-0 force-pushed the feat/cluster_conn branch from 0941b43 to e9af645 Compare October 30, 2024 05:18

hzxuzhonghu approved these changes Oct 30, 2024

View reviewed changes

kmesh-bot assigned hzxuzhonghu Oct 30, 2024

kmesh-bot added the lgtm label Oct 30, 2024

kmesh-bot added the approved label Oct 30, 2024

kmesh-bot merged commit a94c2d1 into kmesh-net:main Oct 30, 2024
9 checks passed

maintain cluster active connection counter for circuit breaker #570

maintain cluster active connection counter for circuit breaker #570

Uh oh!

Conversation

Okabe-Rintarou-0 commented Jul 12, 2024 • edited by hzxuzhonghu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Okabe-Rintarou-0 commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Okabe-Rintarou-0 commented Jul 13, 2024

Uh oh!

Uh oh!

LiZhenCheng9527 commented Jul 13, 2024

Uh oh!

codecov bot commented Jul 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Okabe-Rintarou-0 commented Jul 16, 2024

Uh oh!

Okabe-Rintarou-0 commented Jul 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hzxuzhonghu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu left a comment

Choose a reason for hiding this comment

Uh oh!

kmesh-bot commented Oct 30, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Okabe-Rintarou-0 commented Jul 12, 2024 •

edited by hzxuzhonghu

Loading

Okabe-Rintarou-0 commented Jul 12, 2024 •

edited

Loading

codecov bot commented Jul 13, 2024 •

edited

Loading

Okabe-Rintarou-0 commented Jul 21, 2024 •

edited

Loading