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: refine exists log print with ctx #38080

Merged
merged 1 commit into from
Dec 14, 2024
Merged

enhance: refine exists log print with ctx #38080

merged 1 commit into from
Dec 14, 2024

Conversation

tinswzy
Copy link
Contributor

@tinswzy tinswzy commented Nov 28, 2024

issue: #35917
Refines exists log print with ctx

@sre-ci-robot sre-ci-robot added the size/XL Denotes a PR that changes 500-999 lines. label Nov 28, 2024
@mergify mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels Nov 28, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Nov 28, 2024

@tinswzy cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

@mergify Mergify
Copy link
Contributor

mergify bot commented Nov 28, 2024

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

@tinswzy
Copy link
Contributor Author

tinswzy commented Nov 28, 2024

rerun cpp-unit-test

@tinswzy
Copy link
Contributor Author

tinswzy commented Nov 28, 2024

/run-cpu-e2e

@tinswzy
Copy link
Contributor Author

tinswzy commented Nov 28, 2024

rerun ut

@codecov Codecov
Copy link

codecov bot commented Nov 29, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.92%. Comparing base (e279ccf) to head (22ee07e).
Report is 21 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (e279ccf) and HEAD (22ee07e). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (e279ccf) HEAD (22ee07e)
2 1
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #38080       +/-   ##
===========================================
- Coverage   78.81%   68.92%    -9.90%     
===========================================
  Files        1368      292     -1076     
  Lines      191697    26153   -165544     
===========================================
- Hits       151088    18025   -133063     
+ Misses      35118     8128    -26990     
+ Partials     5491        0     -5491
Components Coverage Δ
Client ∅ <ø> (∅)
Core 68.92% <ø> (ø)
Go ∅ <ø> (∅)

see 1076 files with indirect coverage changes

@tinswzy
Copy link
Contributor Author

tinswzy commented Nov 29, 2024

rerun ut

1 similar comment
@tinswzy
Copy link
Contributor Author

tinswzy commented Nov 29, 2024

rerun ut

@tinswzy tinswzy force-pushed the refine_exists_log_print_using_ctx branch from 57e8238 to 03703cc Compare November 29, 2024 06:56
@mergify Mergify
Copy link
Contributor

mergify bot commented Nov 29, 2024

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

@tinswzy
Copy link
Contributor Author

tinswzy commented Nov 29, 2024

/run-cpu-e2e

@tinswzy
Copy link
Contributor Author

tinswzy commented Nov 29, 2024

rerun ut

@tinswzy
Copy link
Contributor Author

tinswzy commented Nov 29, 2024

/run-cpu-e2e

@tinswzy tinswzy force-pushed the refine_exists_log_print_using_ctx branch from 03703cc to e3d6222 Compare November 30, 2024 01:17
@sre-ci-robot 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 Nov 30, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Nov 30, 2024

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

@tinswzy
Copy link
Contributor Author

tinswzy commented Dec 1, 2024

/run-cpu-e2e

@tinswzy
Copy link
Contributor Author

tinswzy commented Dec 1, 2024

rerun ut

2 similar comments
@tinswzy
Copy link
Contributor Author

tinswzy commented Dec 1, 2024

rerun ut

@tinswzy
Copy link
Contributor Author

tinswzy commented Dec 2, 2024

rerun ut

@tinswzy tinswzy force-pushed the refine_exists_log_print_using_ctx branch from e3d6222 to 7699a7d Compare December 2, 2024 09:51
@tinswzy tinswzy force-pushed the refine_exists_log_print_using_ctx branch 2 times, most recently from ef66a96 to 52da0b3 Compare December 2, 2024 11:20
@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 2, 2024

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

@tinswzy
Copy link
Contributor Author

tinswzy commented Dec 3, 2024

/run-cpu-e2e

@tinswzy tinswzy force-pushed the refine_exists_log_print_using_ctx branch 2 times, most recently from 216149c to 2a064e9 Compare December 4, 2024 03:16
@mergify mergify bot added the ci-passed label Dec 4, 2024
@tinswzy tinswzy force-pushed the refine_exists_log_print_using_ctx branch from 2a064e9 to f603891 Compare December 4, 2024 11:06
@mergify mergify bot removed the ci-passed label Dec 4, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 4, 2024

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

@tinswzy
Copy link
Contributor Author

tinswzy commented Dec 4, 2024

/run-cpu-e2e

@tinswzy tinswzy force-pushed the refine_exists_log_print_using_ctx branch 2 times, most recently from 5bd70fa to f3503a7 Compare December 7, 2024 08:28
@mergify mergify bot added the ci-passed label Dec 7, 2024
@tinswzy tinswzy force-pushed the refine_exists_log_print_using_ctx branch from f3503a7 to 22ee07e Compare December 11, 2024 13:19
@mergify mergify bot added ci-passed and removed ci-passed labels Dec 11, 2024
jaime0815
jaime0815 reviewed Dec 13, 2024
View reviewed changes
internal/proxy/segment.go
@@ -317,7 +317,7 @@ func (sa *segIDAssigner) syncSegments() (bool, error) {
strconv.FormatInt(paramtable.GetNodeID(), 10)).Observe(float64(len(sa.segReqs)))
sa.segReqs = nil

log.Debug("syncSegments call dataCoord.AssignSegmentID", zap.Stringer("request", req))
log.Ctx(context.TODO()).Debug("syncSegments call dataCoord.AssignSegmentID", zap.Stringer("request", req))

resp, err := sa.dataCoord.AssignSegmentID(context.Background(), req)
Copy link
Contributor

Choose a reason for hiding this comment

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

It should use the current method's ctx as a parameter for remote RPC in component communication.

@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: tinswzy

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

@jaime0815
Copy link
Contributor

/lgtm

@sre-ci-robot sre-ci-robot merged commit 27229f7 into milvus-io:master Dec 14, 2024
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaime0815 jaime0815 jaime0815 left review comments

@bigsheeper bigsheeper Awaiting requested review from bigsheeper

@congqixia congqixia Awaiting requested review from congqixia

Assignees

@jaime0815 jaime0815

Labels
approved 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
No milestone
Development

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