-
-
Notifications
You must be signed in to change notification settings - Fork 145
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.
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
Employ etcd for distributed locking #148
Conversation
Stress test for distributed locking based on etcdDescription of test conditions
start := time.Now()
size := 500
sum := 0
var wg gosync.WaitGroup
for i := 0; i < size; i++ {
wg.Add(1)
go func() {
defer wg.Done()
ctx := context.Background()
locker := cli.NewLocker(ctx, sync.Key(t.Name()))
locker.Lock()
sum += 1
locker.Unlock()
}()
}
wg.Wait()
assert.Equal(t, size, sum)
log.Logger.Infof("lock count: %d, elapsed: %s", size, time.Since(start)) Results
|
d079918
to
eb4ac22
Compare
Codecov Report
@@ Coverage Diff @@
## main #148 +/- ##
==========================================
- Coverage 57.05% 57.03% -0.02%
==========================================
Files 33 34 +1
Lines 3106 3191 +85
==========================================
+ Hits 1772 1820 +48
- Misses 1154 1187 +33
- Partials 180 184 +4
Continue to review full report at Codecov.
|
7349022
to
e68830d
Compare
e68830d
to
72cb8ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🔥
Sorry, I am not familiar with ETCD, so there is a delay in reviewing it. 😭 |
@hackerwins Thank you for your PR! Could you elaborate on this PR in the weekly meeting? Because im also not used to etcd, it would be great to hear about this task! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the wonderful work. Thanks to you, I am learning a lot. 🙇♂️
I am trying to fully understand this feature and have made a few comments along the way. Please check.
1e80e29
to
5bf1ad7
Compare
Since we are using lease based lock, we need to introduce fencing token. |
5bf1ad7
to
5909af9
Compare
2f071bf
to
9eee471
Compare
a7ea019
to
80ea20b
Compare
d42cbf1
to
f943bd5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few inquiries and suggestions!
Please answer after checking. :)
In order to allow multiple agents to process client requests, etcd is employed. In standalone mode with only one agent, memory-based locker is used, not etcd.
f943bd5
to
540d2b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 I learned a lot while reviewing. Thank you! :)
In order to allow multiple agents to process client requests, etcd is employed. In standalone mode with only one agent, memory-based locker is used, not etcd.
What this PR does / why we need it:
Employ etcd for distributed locking
In order to allow multiple agents to process client requests, etcd is
employed. In standalone mode with only one agent, memory-based locker is
used, not etcd.
Which issue(s) this PR fixes:
Address #11
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist: