Skip to content

client/v3: add ResumeMutex to concurrency package - #22384

Open
avanish-garg wants to merge 1 commit into
etcd-io:mainfrom
avanish-garg:feature/concurrency-resume-mutex
Open

avanish-garg wants to merge 1 commit into
etcd-io:mainfrom
avanish-garg:feature/concurrency-resume-mutex

Conversation

@avanish-garg

Copy link
Copy Markdown

Fixes #22382

Problem

concurrency.Election has ResumeElection, for reconstructing an election handle after
a process restart (used together with Session's WithLease option for a surviving
lease). concurrency.Mutex has no equivalent -- a restarted process that was holding a
lock has no way to get a working *Mutex handle back for it; it can only start over with
a fresh Lock() call, losing the distinction between "I already own this lock" and "I
need to compete for it."

Change

  • Added ResumeMutex(s *Session, pfx, myKey string, myRev int64) *Mutex, mirroring
    ResumeElection's exact shape and doc-comment style.
  • Added TestResumeMutex to tests/integration/clientv3/concurrency/mutex_test.go,
    mirroring the existing TestResumeElection pattern: locks a mutex, reconstructs a
    second *Mutex handle purely from the session + observed key/revision (simulating a
    process restart), and verifies IsOwner() still recognizes it as the owner and
    Unlock() correctly deletes the real server-side key.

Mutex's myKey/myRev fields are structurally identical to Election's
leaderKey/leaderRev, and IsOwner()/Unlock() only depend on those two values being
set correctly -- they have no dependency on which constructor produced them, so this is a
small, mechanical port of a pattern already proven for Election.

Verified: go build ./client/v3/..., go vet ./client/v3/concurrency/... clean, full
existing concurrency integration test suite passes alongside the new test (11/11,
including TestResumeElection, TestMutexLockSessionExpired, TestMutexUnlock, and the
package's Example* tests).

concurrency.Election already has ResumeElection, for reconstructing an
election handle after a process restart (paired with Session's WithLease
option for a surviving lease). Mutex had no equivalent, so a restarted
process holding a lock had no way to get a working *Mutex handle back for
it -- it could only start over with a fresh Lock() call.

ResumeMutex mirrors ResumeElection's shape exactly: given a session, the
same key prefix, and the previously-observed owner key/revision, it
reconstructs a Mutex that IsOwner() and Unlock() work correctly against.

Signed-off-by: Avanish Garg <gargavanish@gmail.com>
@kubernetes-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: avanish-garg
Once this PR has been reviewed and has the lgtm label, please assign ivanvc for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details 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

@kubernetes-prow

Copy link
Copy Markdown

Hi @avanish-garg. Thanks for your PR.

I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@avanish-garg

Copy link
Copy Markdown
Author

cc @ptabor @fuweid — both of you have prior commits in concurrency/mutex.go and concurrency/election.go (this PR mirrors ResumeElection's existing pattern for Mutex), so tagging in case this area is useful for you to look at. Also, this PR is gated behind needs-ok-to-test — would appreciate an /ok-to-test if it looks reasonable.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

concurrency.ResumeMutex

1 participant