Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

enhance: erase the rpc level when wal is located at same node #38858

Merged
merged 1 commit into from
Feb 5, 2025
Merged

enhance: erase the rpc level when wal is located at same node #38858

merged 1 commit into from
Feb 5, 2025

Conversation

chyezh
Copy link
Contributor

@chyezh chyezh commented Dec 30, 2024

issue: #38399

  • Make the wal scanner interface same with streaming scanner.
  • Use wal if the wal is located at current node.
  • Otherwise fallback the old logic.

@sre-ci-robot sre-ci-robot added area/internal-api size/XXL Denotes a PR that changes 1000+ lines. labels Dec 30, 2024
@mergify mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels Dec 30, 2024
@chyezh chyezh mentioned this pull request Dec 30, 2024
Open
10 tasks
@codecov Codecov
Copy link

codecov bot commented Dec 30, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 84.47205% with 25 lines in your changes missing coverage. Please review.

Project coverage is 80.10%. Comparing base (e61a841) to head (44a42ee).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
...treamingnode/client/handler/handler_client_impl.go 78.84% 7 Missing and 4 partials ⚠️
pkg/streaming/util/message/adaptor/handler.go 80.35% 8 Missing and 3 partials ⚠️
...reamingnode/client/handler/registry/wal_manager.go 80.00% 1 Missing and 1 partial ⚠️
...amingnode/client/handler/consumer/consumer_impl.go 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #38858      +/-   ##
==========================================
- Coverage   80.11%   80.10%   -0.01%     
==========================================
  Files        1465     1464       -1     
  Lines      203083   203120      +37     
==========================================
+ Hits       162700   162712      +12     
- Misses      34576    34603      +27     
+ Partials     5807     5805       -2
Components Coverage Δ
Client 79.45% <ø> (ø)
Core 69.40% <ø> (-0.02%) ⬇️
Go 81.84% <84.37%> (-0.01%) ⬇️
Files with missing lines Coverage Δ
...buted/streaming/internal/consumer/consumer_impl.go 88.14% <100.00%> (+1.38%) ⬆️
...distributed/streaming/internal/consumer/handler.go 100.00% <100.00%> (ø)
...ted/streaming/internal/consumer/message_handler.go 100.00% <100.00%> (ø)
...istributed/streaming/internal/producer/producer.go 92.43% <100.00%> (+4.20%) ⬆️
internal/distributed/streamingnode/service.go 68.13% <ø> (-0.12%) ⬇️
...amingnode/client/handler/producer/producer_impl.go 76.69% <100.00%> (-3.59%) ⬇️
...ode/server/flusher/flusherimpl/channel_lifetime.go 71.27% <100.00%> (ø)
internal/streamingnode/server/server.go 93.93% <100.00%> (+0.18%) ⬆️
...treamingnode/server/wal/adaptor/scanner_adaptor.go 89.51% <100.00%> (ø)
...al/streamingnode/server/walmanager/manager_impl.go 94.28% <100.00%> (+0.11%) ⬆️
... and 5 more

... and 28 files with indirect coverage changes

@chyezh chyezh force-pushed the enhance_enable_querynode_in_streamingnode branch from 8b9bc00 to 1e80731 Compare January 2, 2025 03:56
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 2, 2025

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

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 2, 2025

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

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 2, 2025

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

@chyezh chyezh force-pushed the enhance_enable_querynode_in_streamingnode branch from f4cbd43 to 059bef1 Compare January 15, 2025 02:53
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 15, 2025

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

@chyezh chyezh force-pushed the enhance_enable_querynode_in_streamingnode branch from 059bef1 to 9060f98 Compare January 23, 2025 08:57
@chyezh chyezh added this to the 2.6.0 milestone Jan 23, 2025
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 23, 2025

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

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 23, 2025

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

@chyezh chyezh force-pushed the enhance_enable_querynode_in_streamingnode branch 2 times, most recently from 5562c09 to 0766a73 Compare January 24, 2025 14:38
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 24, 2025

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

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 24, 2025

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

@chyezh chyezh force-pushed the enhance_enable_querynode_in_streamingnode branch from 0766a73 to 21f09f7 Compare January 25, 2025 03:46
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 25, 2025

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

@chyezh
Copy link
Contributor Author

chyezh commented Jan 25, 2025

rerun go-sdk

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 25, 2025

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

@chyezh
enhance: erase the rpc level when wal is located at same node
44a42ee 
- Make the wal scanner interface same with streaming scanner.
- Use wal if the wal is located at current node.
- Otherwise fallback the old logic.

Signed-off-by: chyezh <[email protected]>
@chyezh chyezh force-pushed the enhance_enable_querynode_in_streamingnode branch from 21f09f7 to 44a42ee Compare January 25, 2025 05:32
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 25, 2025

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

@chyezh
Copy link
Contributor Author

chyezh commented Jan 25, 2025

rerun go-sdk

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 25, 2025

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

@chyezh
Copy link
Contributor Author

chyezh commented Jan 25, 2025

rerun go-sdk

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 25, 2025

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

@chyezh
Copy link
Contributor Author

chyezh commented Jan 29, 2025

rerun go-sdk

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 29, 2025

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

@chyezh
Copy link
Contributor Author

chyezh commented Jan 31, 2025

rerun go-sdk

@mergify mergify bot added the ci-passed label Jan 31, 2025
chyezh
chyezh commented Feb 5, 2025
View reviewed changes
pkg/streaming/util/message/message_handler.go

// Close is called after all messages are handled or handling is interrupted.
Close()
}

var _ Handler = ChanMessageHandler(nil)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moving to adaptor package

chyezh
chyezh commented Feb 5, 2025
View reviewed changes
internal/streamingnode/server/wal/scanner.go
@@ -45,27 +43,3 @@ type Scanner interface {
// Return the error same with `Error`
Close() error
}

type HandleParam struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

move to message package

@jaime0815
Copy link
Contributor

/lgtm

@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: chyezh

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:

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 sre-ci-robot merged commit 5669016 into milvus-io:master Feb 5, 2025
20 checks passed
@chyezh chyezh deleted the enhance_enable_querynode_in_streamingnode branch February 6, 2025 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bigsheeper bigsheeper Awaiting requested review from bigsheeper

@congqixia congqixia Awaiting requested review from congqixia

Assignees

@jaime0815 jaime0815

Labels
approved area/internal-api area/test ci-passed dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement lgtm sig/testing size/XXL Denotes a PR that changes 1000+ lines. test/integration integration test
Projects
None yet
Milestone
2.6.0
Development

Successfully merging this pull request may close these issues.
3 participants
@chyezh @jaime0815 @sre-ci-robot