enhance: add rw/ro streaming query node replica management #38677

chyezh · 2024-12-24T02:18:49Z

issue: #38399

Embed the query node into streaming node to make delegator available at streaming node.
The embedded query node has a special server label QUERYNODE_STREAMING-EMBEDDED.
Change the balance strategy to make the channel assigned to streaming node as much as possible.

mergify · 2024-12-24T02:38:53Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-12-24T03:25:42Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-12-24T03:55:50Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

codecov · 2024-12-24T04:40:58Z

Codecov Report

Attention: Patch coverage is 75.53816% with 125 lines in your changes missing coverage. Please review.

Project coverage is 81.14%. Comparing base (e5eb115) to head (1f5b4ca).
Report is 117 commits behind head on master.

Files with missing lines	Patch %	Lines
...ernal/querycoordv2/balance/multi_target_balance.go	0.00%	34 Missing ⚠️
...al/querycoordv2/balance/rowcount_based_balancer.go	70.58%	10 Missing and 5 partials ⚠️
internal/querycoordv2/ops_services.go	23.52%	9 Missing and 4 partials ⚠️
...erycoordv2/balance/channel_level_score_balancer.go	18.18%	8 Missing and 1 partial ⚠️
...ternal/querycoordv2/meta/replica_manager_helper.go	82.35%	8 Missing and 1 partial ⚠️
internal/querycoordv2/server.go	12.50%	5 Missing and 2 partials ⚠️
...dv2/balance/streaming_query_node_channel_helper.go	85.00%	4 Missing and 2 partials ⚠️
internal/querycoordv2/meta/replica_manager.go	87.23%	4 Missing and 2 partials ⚠️
pkg/util/merr/utils.go	0.00%	6 Missing ⚠️
...nternal/querycoordv2/observers/replica_observer.go	90.56%	3 Missing and 2 partials ⚠️
... and 5 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #38677      +/-   ##
==========================================
+ Coverage   81.11%   81.14%   +0.02%     
==========================================
  Files        1395     1397       +2     
  Lines      197298   197712     +414     
==========================================
+ Hits       160037   160427     +390     
- Misses      31623    31646      +23     
- Partials     5638     5639       +1

Components	Coverage Δ
Client	`79.53% <ø> (ø)`
Core	`69.64% <ø> (ø)`
Go	`83.06% <75.53%> (+0.02%)`	⬆️

Files with missing lines	Coverage Δ
...ernal/querycoordv2/balance/score_based_balancer.go	`98.78% <100.00%> (+0.71%)`	⬆️
internal/querycoordv2/checkers/channel_checker.go	`87.35% <100.00%> (+0.22%)`	⬆️
internal/querycoordv2/meta/replica.go	`100.00% <100.00%> (ø)`
internal/querycoordv2/session/node_manager.go	`100.00% <100.00%> (ø)`
...al/streamingcoord/server/balancer/balancer_impl.go	`77.99% <100.00%> (+0.76%)`	⬆️
internal/streamingcoord/server/server.go	`72.22% <100.00%> (+0.52%)`	⬆️
internal/util/sessionutil/session_util.go	`76.51% <ø> (ø)`
...ode/server/flusher/flusherimpl/channel_lifetime.go	`71.27% <0.00%> (ø)`
...al/coordinator/snmanager/streaming_node_manager.go	`94.73% <94.73%> (ø)`
internal/querycoordv2/balance/balance.go	`91.20% <70.00%> (-2.91%)`	⬇️
... and 12 more

... and 30 files with indirect coverage changes

mergify · 2024-12-24T04:42:29Z

@chyezh go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-12-26T14:09:22Z

@chyezh go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-12-26T14:34:49Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-12-27T16:39:01Z

@chyezh go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-12-28T12:49:14Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-12-29T03:20:18Z

@chyezh go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-12-29T06:53:18Z

@chyezh go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-12-29T07:53:26Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-12-29T10:43:59Z

@chyezh go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-12-29T11:09:37Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2025-01-10T03:36:53Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

Signed-off-by: chyezh <[email protected]>

mergify · 2025-01-10T04:59:35Z

@chyezh go-sdk check failed, comment rerun go-sdk can trigger the job again.

chyezh · 2025-01-10T05:46:26Z

rerun go-sdk

mergify · 2025-01-10T06:38:32Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

chyezh · 2025-01-10T07:07:35Z

/run-cpu-e2e

mergify · 2025-01-10T08:33:43Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

chyezh · 2025-01-10T09:21:47Z

/run-cpu-e2e

chyezh · 2025-01-10T09:48:05Z

/run-cpu-e2e

mergify · 2025-01-10T10:55:55Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

chyezh · 2025-01-10T11:50:53Z

/run-cpu-e2e

mergify · 2025-01-10T12:58:20Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

chyezh · 2025-01-10T13:11:08Z

/run-cpu-e2e

jaime0815 · 2025-01-21T12:13:50Z

internal/coordinator/snmanager/streaming_node_manager.go

+			s.cond.L.Unlock()
+			return nil
+		}); err != nil {
+			return err


Although here only context cancel error, I still suggest adding a log before the return

jaime0815 · 2025-01-21T12:28:40Z

internal/coordinator/snmanager/streaming_node_manager.go

+
+// StreamingNodeManager is a manager for manage the querynode that embedded into streaming node.
+// StreamingNodeManager is exclusive with ResourceManager.
+type StreamingNodeManager struct {


Use StreamingNodeObserver instead of StreamingNodeManager

liliu-z

/lgtm
/approve

liliu-z · 2025-01-23T08:33:50Z

cmd/milvus/util.go

@@ -175,6 +177,10 @@ func GetMilvusRoles(args []string, flags *flag.FlagSet) *roles.MilvusRoles {
 		role.EnableIndexNode = enableIndexNode
 		role.EnableProxy = enableProxy
 		role.EnableStreamingNode = enableStreamingNode


Why we don't check IsStreamingServiceEnabled here?

If enableStreamingNode is setup, IsStreamingServiceEnabled is always true for mixture.
Because the streaming service must be enabled at 2.6, we will remove all checker before 2.6 release.
But here is huge amount related-unittest modification, the removing pr is deferred.

liliu-z · 2025-01-23T08:35:05Z

cmd/milvus/util.go

@@ -150,7 +150,9 @@ func GetMilvusRoles(args []string, flags *flag.FlagSet) *roles.MilvusRoles {
 		role.EnableIndexNode = true
 	case typeutil.StreamingNodeRole:
 		streamingutil.MustEnableStreamingService()


Redundant check

liliu-z · 2025-01-23T08:49:58Z

internal/coordinator/snmanager/streaming_node_manager.go

+	"github.com/milvus-io/milvus/pkg/util/typeutil"
+)
+
+var StaticStreamingNodeManager = newStreamingNodeManager()


Why don't use Sync.Once. Plz CMIIW, this will be inited no matter what kind of nodes

it's a static-initialized variable, no busy operation happened when init, no concurrent issue here.
So sync.Once is not applied, but a redundant dead goroutine here.
It should be only available at coordinator, I will fix it.

liliu-z · 2025-01-23T12:49:27Z

internal/util/streamingutil/env.go

+// EnableEmbededQueryNode set server labels for embedded query node.
+func EnableEmbededQueryNode() {
+	MustEnableStreamingService()
+	os.Setenv(sessionutil.SupportedLabelPrefix+sessionutil.LabelStreamingNodeEmbeddedQueryNode, "1")


Curios why we set a env variable instead of a global variable

both env and global var is ok here.
i will modify to use global var at another pr.

sre-ci-robot · 2025-01-24T08:54:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chyezh, liliu-z

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~~ [liliu-z]

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

sre-ci-robot added area/internal-api size/XL Denotes a PR that changes 500-999 lines. labels Dec 24, 2024

sre-ci-robot requested review from congqixia and jiaoew1991 December 24, 2024 02:19

mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels Dec 24, 2024

chyezh force-pushed the enhance_embed_querynode_in_streamingnode branch from c3ce6ad to 0c32ff6 Compare December 24, 2024 02:44

chyezh force-pushed the enhance_embed_querynode_in_streamingnode branch from 5d38958 to 781ff46 Compare December 26, 2024 13:36

chyezh force-pushed the enhance_embed_querynode_in_streamingnode branch from 781ff46 to 3f613f6 Compare December 27, 2024 16:11

chyezh force-pushed the enhance_embed_querynode_in_streamingnode branch 2 times, most recently from 9db9bcf to 3953e7c Compare December 28, 2024 11:47

chyezh force-pushed the enhance_embed_querynode_in_streamingnode branch 2 times, most recently from 70a9f34 to 2544e08 Compare December 29, 2024 02:46

sre-ci-robot added size/XXL Denotes a PR that changes 1000+ lines. and removed size/XL Denotes a PR that changes 500-999 lines. labels Dec 29, 2024

chyezh force-pushed the enhance_embed_querynode_in_streamingnode branch from 413fd4b to 1546f5a Compare December 29, 2024 11:16

chyezh force-pushed the enhance_embed_querynode_in_streamingnode branch from 99e8552 to e22ad87 Compare January 7, 2025 13:09

mergify bot added the ci-passed label Jan 7, 2025

chyezh force-pushed the enhance_embed_querynode_in_streamingnode branch from e22ad87 to 789b8dc Compare January 10, 2025 02:58

mergify bot removed the ci-passed label Jan 10, 2025

chyezh force-pushed the enhance_embed_querynode_in_streamingnode branch 2 times, most recently from 5dfd0d7 to 1f5b4ca Compare January 10, 2025 04:22

enhance: add rw/ro streaming query node replica management

1f5b4ca

Signed-off-by: chyezh <[email protected]>

mergify bot added the ci-passed label Jan 10, 2025

jaime0815 reviewed Jan 21, 2025

View reviewed changes

chyezh added this to the 2.6.0 milestone Jan 23, 2025

liliu-z reviewed Jan 24, 2025

View reviewed changes

sre-ci-robot assigned liliu-z Jan 24, 2025

sre-ci-robot added the lgtm label Jan 24, 2025

sre-ci-robot added the approved label Jan 24, 2025

sre-ci-robot merged commit c84a074 into milvus-io:master Jan 24, 2025
19 of 20 checks passed

chyezh deleted the enhance_embed_querynode_in_streamingnode branch January 25, 2025 04:13

enhance: add rw/ro streaming query node replica management #38677

enhance: add rw/ro streaming query node replica management #38677

Conversation

chyezh commented Dec 24, 2024 • edited Loading

mergify bot commented Dec 24, 2024

mergify bot commented Dec 24, 2024

mergify bot commented Dec 24, 2024

codecov bot commented Dec 24, 2024 • edited Loading

Codecov Report

mergify bot commented Dec 24, 2024

mergify bot commented Dec 26, 2024

mergify bot commented Dec 26, 2024

mergify bot commented Dec 27, 2024

mergify bot commented Dec 28, 2024

mergify bot commented Dec 29, 2024

mergify bot commented Dec 29, 2024

mergify bot commented Dec 29, 2024

mergify bot commented Dec 29, 2024

mergify bot commented Dec 29, 2024

mergify bot commented Jan 10, 2025

mergify bot commented Jan 10, 2025

chyezh commented Jan 10, 2025

mergify bot commented Jan 10, 2025

chyezh commented Jan 10, 2025

mergify bot commented Jan 10, 2025

chyezh commented Jan 10, 2025

chyezh commented Jan 10, 2025

mergify bot commented Jan 10, 2025

chyezh commented Jan 10, 2025

mergify bot commented Jan 10, 2025

chyezh commented Jan 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liliu-z left a 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

chyezh Jan 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sre-ci-robot commented Jan 24, 2025

chyezh commented Dec 24, 2024 •

edited

Loading

codecov bot commented Dec 24, 2024 •

edited

Loading

chyezh Jan 25, 2025 •

edited

Loading