enhance: Mark cgo thread with tag name #38000
Conversation
congqixia
changed the title
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: congqixia 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 |
mergify
bot
added
dco-passed
| conc.WithPreHandler(runtime.LockOSThread), // lock os thread for cgo thread disposal | ||
| conc.WithPreHandler(func() { | ||
| runtime.LockOSThread() | ||
| C.SetThreadName(cgoTagDynamic) |
There was a problem hiding this comment.
Can we make this a mandatory param?
There was a problem hiding this comment.
WithPreHandler？Since its a general worker pool, it is not recommend to make CGO related params mandatory
|
@congqixia go-sdk check failed, comment |
|
@congqixia E2e jenkins job failed, comment |
Related to milvus-io#37999 This PR add `SetThreadName` API for marking cgo thread and utilize it when initializing cgo worker. Signed-off-by: Congqi Xia <[email protected]>
Signed-off-by: Congqi Xia <[email protected]>
congqixia
force-pushed
the
enhance/mark_cgo_thread_name
branch
from
5ee2f2d to
b339976
Compare
|
@congqixia go-sdk check failed, comment |
|
@congqixia cpp-unit-test check failed, comment |
|
@congqixia E2e jenkins job failed, comment |
|
rerun cpp-unit-test |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #38000 +/- ##
===========================================
+ Coverage 68.87% 81.07% +12.19%
===========================================
Files 289 1360 +1071
Lines 25481 190781 +165300
===========================================
+ Hits 17551 154671 +137120
- Misses 7930 30627 +22697
- Partials 0 5483 +5483
|
|
rerun go-sdk |
|
/run-cpu-e2e |
|
@congqixia E2e jenkins job failed, comment |
|
/run-cpu-e2e |
There was a problem hiding this comment.
Talked offline, we can merge this code for now but some questions need to be taken care of:
- How to name OS thread in go side
- Name need to be passed to TP's constructor after we find a way to name both cgo threads and go side threads
- Why these many singleton, can we cut down some of them?
/lgtm
Related to milvus-io#37999 This PR add `SetThreadName` API for marking cgo thread and utilize it when initializing cgo worker. --------- Signed-off-by: Congqi Xia <[email protected]>
Related to milvus-io#37999 This PR add `SetThreadName` API for marking cgo thread and utilize it when initializing cgo worker. --------- Signed-off-by: Congqi Xia <[email protected]>
Related to #37999
This PR add
SetThreadNameAPI for marking cgo thread and utilize it when initializing cgo worker.