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

[Enhancement]: Refine the lock usage of segment LoadStateLock #39205

Closed
1 task done
congqixia opened this issue Jan 13, 2025 · 0 comments
Closed
1 task done

[Enhancement]: Refine the lock usage of segment LoadStateLock #39205

congqixia opened this issue Jan 13, 2025 · 0 comments
Assignees
@congqixia
Labels
kind/enhancement Issues or changes related to enhancement

Comments

@congqixia
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

What would you like to be added?

There are currently two meanings for LoadStateLock

  • Pin segment at certain state
  • RLock on segment ptr prevent being releasing during cgo

We'd like to unify these two operators into one: pinning segment state, which could also preventing segment from being releasing

Why is this needed?

Holding the rlock doing cgo (which could carry io operations, say WarmupChunkCache) is not a good implementation. This enhancement could simplify the "lock" meaning and make lock duration as small as possible

Anything else?

No response
@congqixia congqixia added the kind/enhancement Issues or changes related to enhancement label Jan 13, 2025
@congqixia congqixia self-assigned this Jan 13, 2025
congqixia added a commit to congqixia/milvus that referenced this issue Jan 13, 2025
@congqixia
enhance: Unify LoadStateLock RLock & PinIf
5b87cec 
Related to milvus-io#39205

This PR merge `RLock` & `PinIfNotReleased` into `PinIf` function
preventing segment being released before any Read operation finished.

Signed-off-by: Congqi Xia <[email protected]>
@congqixia congqixia mentioned this issue Jan 13, 2025
Merged
congqixia added a commit to congqixia/milvus that referenced this issue Jan 13, 2025
@congqixia
enhance: Unify LoadStateLock RLock & PinIf
c7a6b39 
Related to milvus-io#39205

This PR merge `RLock` & `PinIfNotReleased` into `PinIf` function
preventing segment being released before any Read operation finished.

Signed-off-by: Congqi Xia <[email protected]>
congqixia added a commit to congqixia/milvus that referenced this issue Jan 14, 2025
@congqixia
enhance: Unify LoadStateLock RLock & PinIf
6f6923c 
Related to milvus-io#39205

This PR merge `RLock` & `PinIfNotReleased` into `PinIf` function
preventing segment being released before any Read operation finished.

Signed-off-by: Congqi Xia <[email protected]>
sre-ci-robot pushed a commit that referenced this issue Jan 14, 2025
@congqixia
enhance: Unify LoadStateLock RLock & PinIf (#39206)
d89768f 
Related to #39205

This PR merge `RLock` & `PinIfNotReleased` into `PinIf` function
preventing segment being released before any Read operation finished.

---------

Signed-off-by: Congqi Xia <[email protected]>
congqixia added a commit to congqixia/milvus that referenced this issue Jan 14, 2025
@congqixia
enhance: Unify LoadStateLock RLock & PinIf (milvus-io#39206)
76773bf 
Related to milvus-io#39205

This PR merge `RLock` & `PinIfNotReleased` into `PinIf` function
preventing segment being released before any Read operation finished.

---------

Signed-off-by: Congqi Xia <[email protected]>
@congqixia congqixia mentioned this issue Jan 14, 2025
Merged
congqixia added a commit to congqixia/milvus that referenced this issue Jan 15, 2025
@congqixia
enhance: Unify LoadStateLock RLock & PinIf (milvus-io#39206)
e0f02e1 
Related to milvus-io#39205

This PR merge `RLock` & `PinIfNotReleased` into `PinIf` function
preventing segment being released before any Read operation finished.

---------

Signed-off-by: Congqi Xia <[email protected]>
@congqixia congqixia mentioned this issue Jan 15, 2025
Merged
congqixia added a commit to congqixia/milvus that referenced this issue Jan 15, 2025
@congqixia
enhance: Log error instead of panicking if load lock wait timeout
3408d2f 
Related to milvus-io#39205
Previous PR milvus-io#39206

This PR change wait timeout behavior to log error and return to avoid
making other collection read failure in only some collections have
deadlock

Signed-off-by: Congqi Xia <[email protected]>
@congqixia congqixia mentioned this issue Jan 15, 2025
Merged
congqixia added a commit to congqixia/milvus that referenced this issue Jan 15, 2025
@congqixia
enhance: Log error instead of panicking if load lock wait timeout
0ce7ae0 
Related to milvus-io#39205
Previous PR milvus-io#39206

This PR change wait timeout behavior to log error and return to avoid
making other collection read failure in only some collections have
deadlock

Signed-off-by: Congqi Xia <[email protected]>
congqixia added a commit to congqixia/milvus that referenced this issue Jan 15, 2025
@congqixia
enhance: Log error instead of panicking if load lock wait timeout
157f5ce 
Related to milvus-io#39205
Previous PR milvus-io#39206

This PR change wait timeout behavior to log error and return to avoid
making other collection read failure in only some collections have
deadlock

Signed-off-by: Congqi Xia <[email protected]>
sre-ci-robot pushed a commit that referenced this issue Jan 15, 2025
@congqixia
enhance: [2.5] Unify LoadStateLock RLock & PinIf (#39206) (#39255)
b658467 
Cherry-pick from master
pr: #39206 #39308
Related to #39205

This PR merge `RLock` & `PinIfNotReleased` into `PinIf` function
preventing segment being released before any Read operation finished.

---------

---------

Signed-off-by: Congqi Xia <[email protected]>
sre-ci-robot pushed a commit that referenced this issue Jan 15, 2025
@congqixia
enhance: [2.4] Unify LoadStateLock RLock & PinIf (#39206) (#39290)
ca7ecdb 
Cherry-pick from master
pr: #39206 #39308
Related to #39205

This PR merge `RLock` & `PinIfNotReleased` into `PinIf` function
preventing segment being released before any Read operation finished.

---------

---------

Signed-off-by: Congqi Xia <[email protected]>
sre-ci-robot pushed a commit that referenced this issue Jan 15, 2025
@congqixia
enhance: Log error instead of panicking if load lock wait timeout (#3…
57e5652 
…9308)

Related to #39205
Previous PR #39206

This PR change wait timeout behavior to log error and return to avoid
making other collection read failure in only some collections have
deadlock

Signed-off-by: Congqi Xia <[email protected]>
gifi-siby pushed a commit to gifi-siby/milvus that referenced this issue Jan 16, 2025
@congqixia @gifi-siby
enhance: Unify LoadStateLock RLock & PinIf (milvus-io#39206)
a4fd1bd 
Related to milvus-io#39205

This PR merge `RLock` & `PinIfNotReleased` into `PinIf` function
preventing segment being released before any Read operation finished.

---------

Signed-off-by: Congqi Xia <[email protected]>
gifi-siby pushed a commit to gifi-siby/milvus that referenced this issue Jan 16, 2025
@congqixia @gifi-siby
enhance: Log error instead of panicking if load lock wait timeout (mi…
f6e4283 
…lvus-io#39308)

Related to milvus-io#39205
Previous PR milvus-io#39206

This PR change wait timeout behavior to log error and return to avoid
making other collection read failure in only some collections have
deadlock

Signed-off-by: Congqi Xia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@congqixia congqixia

Labels
kind/enhancement Issues or changes related to enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@congqixia