-
Notifications
You must be signed in to change notification settings - Fork 1
Adds configurable leases to locks to support lock expiry #34
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
base: master
Are you sure you want to change the base?
Changes from all commits
a72c6f7
c761366
e2deede
2291934
5d7733a
6759e05
0b321c2
e35d304
c38311c
cd7a20a
4d6134d
d0da403
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| main | ||
| .vscode | ||
| .vscode/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,27 @@ | ||
| package lockservice | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "sync" | ||
| "time" | ||
|
|
||
| "github.com/rs/zerolog" | ||
| ) | ||
|
|
||
| // SafeLockMap is the lockserver's data structure | ||
| type SafeLockMap struct { | ||
| LockMap map[string]string | ||
| Mutex sync.Mutex | ||
| LockMap map[string]*LockMapEntry | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| LeaseDuration time.Duration | ||
| Mutex sync.Mutex | ||
| } | ||
|
|
||
| // LockMapEntry defines the structure for objects placed | ||
| // in the LockMap. It consists of the owner of the lock | ||
| // that is acquired and the timestamp at which the | ||
| // acquisition took place. | ||
| type LockMapEntry struct { | ||
| owner string | ||
| timestamp time.Time | ||
| } | ||
|
|
||
| // SimpleConfig implements Config. | ||
|
|
@@ -87,6 +99,14 @@ func (sd *LockDescriptor) Owner() string { | |
| return sd.UserID | ||
| } | ||
|
|
||
| // NewLockMapEntry returns an instance of a LockMapEntry | ||
| func NewLockMapEntry(owner string, timestamp time.Time) *LockMapEntry { | ||
| return &LockMapEntry{ | ||
| owner: owner, | ||
| timestamp: timestamp, | ||
| } | ||
| } | ||
|
|
||
| // NewSimpleConfig returns an instance of the SimpleConfig. | ||
| func NewSimpleConfig(IPAddr, PortAddr string) *SimpleConfig { | ||
| return &SimpleConfig{ | ||
|
|
@@ -111,64 +131,108 @@ func NewObjectDescriptor(ObjectID string) *ObjectDescriptor { | |
| } | ||
|
|
||
| // NewSimpleLockService creates and returns a new lock service ready to use. | ||
| func NewSimpleLockService(log zerolog.Logger) *SimpleLockService { | ||
| func NewSimpleLockService(log zerolog.Logger, leaseDuration time.Duration) *SimpleLockService { | ||
| safeLockMap := &SafeLockMap{ | ||
| LockMap: make(map[string]string), | ||
| LockMap: make(map[string]*LockMapEntry), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this have to be a pointer? |
||
| } | ||
| safeLockMap.LeaseDuration = leaseDuration | ||
| return &SimpleLockService{ | ||
| log: log, | ||
| lockMap: safeLockMap, | ||
| } | ||
| } | ||
| func fmtDuration(d time.Duration) string { | ||
| d = d.Round(time.Minute) | ||
| h := d / time.Hour | ||
| d -= h * time.Hour | ||
| ms := d / time.Microsecond | ||
| return fmt.Sprintf("%02d:%02d", h, ms) | ||
| } | ||
|
|
||
| // hasLeaseExpired returns true if the lease for a lock has expired and | ||
| // false if the lease is still valid | ||
| func hasLeaseExpired(timestamp time.Time, lease time.Duration) bool { | ||
| if time.Now().Sub(timestamp) > lease { | ||
| return true | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // Acquire function lets a client acquire a lock on an object. | ||
| // This lock is valid for a fixed duration that is set in the SafeLockMap.LeaseDuration | ||
| // field. Beyond this duration, the lock has expired and the entity that owned the lock | ||
| // for this period can no longer release it. The lock is open for acquisition after it | ||
| // has expired. | ||
| func (ls *SimpleLockService) Acquire(sd Descriptors) error { | ||
| ls.lockMap.Mutex.Lock() | ||
| if _, ok := ls.lockMap.LockMap[sd.ID()]; ok { | ||
|
|
||
| timestamp := ls.lockMap.LockMap[sd.ID()].timestamp | ||
| duration := ls.lockMap.LeaseDuration | ||
| // If the lock is not present in the LockMap or | ||
| // the lock has expired, then allow the acquisition | ||
| if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || hasLeaseExpired(timestamp, duration) { | ||
| ls.lockMap.LockMap[sd.ID()] = NewLockMapEntry(sd.Owner(), time.Now()) | ||
| ls.lockMap.Mutex.Unlock() | ||
| ls. | ||
| log. | ||
| Debug(). | ||
| Str("descriptor", sd.ID()). | ||
| Msg("can't acquire, already been acquired") | ||
| return ErrFileacquired | ||
| Str("owner", ls.lockMap.LockMap[sd.ID()].owner). | ||
| Time("timestamp", ls.lockMap.LockMap[sd.ID()].timestamp). | ||
| Msg("locked") | ||
| return nil | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please explain.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the if condition has changed. The new if condition is essential NOT of the previous one. If you see the additions at like #188, it contains what has been removed here. |
||
| } | ||
| ls.lockMap.LockMap[sd.ID()] = sd.Owner() | ||
| ls.lockMap.Mutex.Unlock() | ||
| // Since the lock is already acquired, return an error | ||
| ls. | ||
| log. | ||
| Debug(). | ||
| Str("descriptor", sd.ID()). | ||
| Str("owner", sd.Owner()). | ||
| Msg("locked") | ||
| return nil | ||
| Msg("can't acquire, already been acquired") | ||
| return ErrFileacquired | ||
|
|
||
| } | ||
|
|
||
| // Release lets a client to release a lock on an object. | ||
| func (ls *SimpleLockService) Release(sd Descriptors) error { | ||
| ls.lockMap.Mutex.Lock() | ||
| timestamp := ls.lockMap.LockMap[sd.ID()].timestamp | ||
| duration := ls.lockMap.LeaseDuration | ||
| // Only the entity that posseses the lock for this object | ||
| // is allowed to release the lock | ||
| if ls.lockMap.LockMap[sd.ID()] == sd.Owner() { | ||
| delete(ls.lockMap.LockMap, sd.ID()) | ||
| if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok { | ||
| // trying to release an unacquired lock | ||
| ls. | ||
| log. | ||
| Debug(). | ||
| Str("descriptor", sd.ID()). | ||
| Str("owner", sd.Owner()). | ||
| Msg("released") | ||
| Msg("can't release, hasn't been acquired") | ||
| ls.lockMap.Mutex.Unlock() | ||
| return nil | ||
| } else if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok { | ||
| return ErrCantReleaseFile | ||
|
|
||
| } else if hasLeaseExpired(timestamp, duration) { | ||
| // lease expired | ||
| ls. | ||
| log. | ||
| Debug(). | ||
| Str("descriptor", sd.ID()). | ||
| Msg("can't release, hasn't been acquired") | ||
| Msg("can't release, lease of lock has expired") | ||
| ls.lockMap.Mutex.Unlock() | ||
| return ErrCantReleaseFile | ||
|
|
||
| } else if ls.lockMap.LockMap[sd.ID()].owner == sd.Owner() { | ||
| // conditions satisfied, lock is released | ||
| delete(ls.lockMap.LockMap, sd.ID()) | ||
| ls. | ||
| log. | ||
| Debug(). | ||
| Str("descriptor", sd.ID()). | ||
| Str("owner", sd.Owner()). | ||
| Msg("released") | ||
| ls.lockMap.Mutex.Unlock() | ||
| return nil | ||
| } else { | ||
| // trying to release a lock that you don't own | ||
| ls. | ||
| log. | ||
| Debug(). | ||
|
|
@@ -186,14 +250,14 @@ func (ls *SimpleLockService) Release(sd Descriptors) error { | |
| func (ls *SimpleLockService) CheckAcquired(sd Descriptors) (string, bool) { | ||
| ls.lockMap.Mutex.Lock() | ||
| id := sd.ID() | ||
| if owner, ok := ls.lockMap.LockMap[id]; ok { | ||
| if entry, ok := ls.lockMap.LockMap[id]; ok { | ||
suraj44 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ls.lockMap.Mutex.Unlock() | ||
| ls. | ||
| log. | ||
| Debug(). | ||
| Str("descriptor", id). | ||
| Msg("checkacquire success") | ||
| return owner, true | ||
| return entry.owner, true | ||
| } | ||
| ls. | ||
| log. | ||
|
|
||
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.
Why?
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.
So i changed the test that checks if session expiry works to instead check for lock expiry.
This test highlighted the redundancy for either lock expiry / session expiry. I feel like only one of them is needed. Having both is a bit redundant imo. What do you think @SUMUKHA-PK ?
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.
Sure, ok