rpc/lock: Make sure to capitalize for proper marshalling. (#3544)

Distributed setup stopped working for certain
types of operations `6d10f4c19af6861e4de1b22ac20a3e5136f69d67`

This is a regression.

Fixes #3543
master
Harshavardhana 8 years ago committed by GitHub
parent a091fe3ed6
commit e1142e99f2
  1. 58
      cmd/lock-rpc-server.go
  2. 2
      cmd/lock-rpc-server_test.go
  3. 4
      cmd/rpc-common.go

@ -137,14 +137,14 @@ func (l *lockServer) Lock(args *LockArgs, reply *bool) error {
if err := args.IsAuthenticated(); err != nil { if err := args.IsAuthenticated(); err != nil {
return err return err
} }
_, *reply = l.lockMap[args.dsyncLockArgs.Resource] _, *reply = l.lockMap[args.LockArgs.Resource]
if !*reply { // No locks held on the given name, so claim write lock if !*reply { // No locks held on the given name, so claim write lock
l.lockMap[args.dsyncLockArgs.Resource] = []lockRequesterInfo{ l.lockMap[args.LockArgs.Resource] = []lockRequesterInfo{
{ {
writer: true, writer: true,
node: args.dsyncLockArgs.ServerAddr, node: args.LockArgs.ServerAddr,
rpcPath: args.dsyncLockArgs.ServiceEndpoint, rpcPath: args.LockArgs.ServiceEndpoint,
uid: args.dsyncLockArgs.UID, uid: args.LockArgs.UID,
timestamp: time.Now().UTC(), timestamp: time.Now().UTC(),
timeLastCheck: time.Now().UTC(), timeLastCheck: time.Now().UTC(),
}, },
@ -162,14 +162,14 @@ func (l *lockServer) Unlock(args *LockArgs, reply *bool) error {
return err return err
} }
var lri []lockRequesterInfo var lri []lockRequesterInfo
if lri, *reply = l.lockMap[args.dsyncLockArgs.Resource]; !*reply { // No lock is held on the given name if lri, *reply = l.lockMap[args.LockArgs.Resource]; !*reply { // No lock is held on the given name
return fmt.Errorf("Unlock attempted on an unlocked entity: %s", args.dsyncLockArgs.Resource) return fmt.Errorf("Unlock attempted on an unlocked entity: %s", args.LockArgs.Resource)
} }
if *reply = isWriteLock(lri); !*reply { // Unless it is a write lock if *reply = isWriteLock(lri); !*reply { // Unless it is a write lock
return fmt.Errorf("Unlock attempted on a read locked entity: %s (%d read locks active)", args.dsyncLockArgs.Resource, len(lri)) return fmt.Errorf("Unlock attempted on a read locked entity: %s (%d read locks active)", args.LockArgs.Resource, len(lri))
} }
if !l.removeEntry(args.dsyncLockArgs.Resource, args.dsyncLockArgs.UID, &lri) { if !l.removeEntry(args.LockArgs.Resource, args.LockArgs.UID, &lri) {
return fmt.Errorf("Unlock unable to find corresponding lock for uid: %s", args.dsyncLockArgs.UID) return fmt.Errorf("Unlock unable to find corresponding lock for uid: %s", args.LockArgs.UID)
} }
return nil return nil
} }
@ -183,18 +183,18 @@ func (l *lockServer) RLock(args *LockArgs, reply *bool) error {
} }
lrInfo := lockRequesterInfo{ lrInfo := lockRequesterInfo{
writer: false, writer: false,
node: args.dsyncLockArgs.ServerAddr, node: args.LockArgs.ServerAddr,
rpcPath: args.dsyncLockArgs.ServiceEndpoint, rpcPath: args.LockArgs.ServiceEndpoint,
uid: args.dsyncLockArgs.UID, uid: args.LockArgs.UID,
timestamp: time.Now().UTC(), timestamp: time.Now().UTC(),
timeLastCheck: time.Now().UTC(), timeLastCheck: time.Now().UTC(),
} }
if lri, ok := l.lockMap[args.dsyncLockArgs.Resource]; ok { if lri, ok := l.lockMap[args.LockArgs.Resource]; ok {
if *reply = !isWriteLock(lri); *reply { // Unless there is a write lock if *reply = !isWriteLock(lri); *reply { // Unless there is a write lock
l.lockMap[args.dsyncLockArgs.Resource] = append(l.lockMap[args.dsyncLockArgs.Resource], lrInfo) l.lockMap[args.LockArgs.Resource] = append(l.lockMap[args.LockArgs.Resource], lrInfo)
} }
} else { // No locks held on the given name, so claim (first) read lock } else { // No locks held on the given name, so claim (first) read lock
l.lockMap[args.dsyncLockArgs.Resource] = []lockRequesterInfo{lrInfo} l.lockMap[args.LockArgs.Resource] = []lockRequesterInfo{lrInfo}
*reply = true *reply = true
} }
return nil return nil
@ -208,14 +208,14 @@ func (l *lockServer) RUnlock(args *LockArgs, reply *bool) error {
return err return err
} }
var lri []lockRequesterInfo var lri []lockRequesterInfo
if lri, *reply = l.lockMap[args.dsyncLockArgs.Resource]; !*reply { // No lock is held on the given name if lri, *reply = l.lockMap[args.LockArgs.Resource]; !*reply { // No lock is held on the given name
return fmt.Errorf("RUnlock attempted on an unlocked entity: %s", args.dsyncLockArgs.Resource) return fmt.Errorf("RUnlock attempted on an unlocked entity: %s", args.LockArgs.Resource)
} }
if *reply = !isWriteLock(lri); !*reply { // A write-lock is held, cannot release a read lock if *reply = !isWriteLock(lri); !*reply { // A write-lock is held, cannot release a read lock
return fmt.Errorf("RUnlock attempted on a write locked entity: %s", args.dsyncLockArgs.Resource) return fmt.Errorf("RUnlock attempted on a write locked entity: %s", args.LockArgs.Resource)
} }
if !l.removeEntry(args.dsyncLockArgs.Resource, args.dsyncLockArgs.UID, &lri) { if !l.removeEntry(args.LockArgs.Resource, args.LockArgs.UID, &lri) {
return fmt.Errorf("RUnlock unable to find corresponding read lock for uid: %s", args.dsyncLockArgs.UID) return fmt.Errorf("RUnlock unable to find corresponding read lock for uid: %s", args.LockArgs.UID)
} }
return nil return nil
} }
@ -227,11 +227,11 @@ func (l *lockServer) ForceUnlock(args *LockArgs, reply *bool) error {
if err := args.IsAuthenticated(); err != nil { if err := args.IsAuthenticated(); err != nil {
return err return err
} }
if len(args.dsyncLockArgs.UID) != 0 { if len(args.LockArgs.UID) != 0 {
return fmt.Errorf("ForceUnlock called with non-empty UID: %s", args.dsyncLockArgs.UID) return fmt.Errorf("ForceUnlock called with non-empty UID: %s", args.LockArgs.UID)
} }
if _, ok := l.lockMap[args.dsyncLockArgs.Resource]; ok { // Only clear lock when set if _, ok := l.lockMap[args.LockArgs.Resource]; ok { // Only clear lock when set
delete(l.lockMap, args.dsyncLockArgs.Resource) // Remove the lock (irrespective of write or read lock) delete(l.lockMap, args.LockArgs.Resource) // Remove the lock (irrespective of write or read lock)
} }
*reply = true *reply = true
return nil return nil
@ -245,17 +245,17 @@ func (l *lockServer) Expired(args *LockArgs, reply *bool) error {
return err return err
} }
// Lock found, proceed to verify if belongs to given uid. // Lock found, proceed to verify if belongs to given uid.
if lri, ok := l.lockMap[args.dsyncLockArgs.Resource]; ok { if lri, ok := l.lockMap[args.LockArgs.Resource]; ok {
// Check whether uid is still active // Check whether uid is still active
for _, entry := range lri { for _, entry := range lri {
if entry.uid == args.dsyncLockArgs.UID { if entry.uid == args.LockArgs.UID {
*reply = false // When uid found, lock is still active so return not expired. *reply = false // When uid found, lock is still active so return not expired.
return nil // When uid found *reply is set to true. return nil // When uid found *reply is set to true.
} }
} }
} }
// When we get here lock is no longer active due to either args.dsyncLockArgs.Resource // When we get here lock is no longer active due to either args.LockArgs.Resource
// being absent from map or uid not found for given args.dsyncLockArgs.Resource // being absent from map or uid not found for given args.LockArgs.Resource
*reply = true *reply = true
return nil return nil
} }

@ -350,7 +350,7 @@ func TestLockRpcServerForceUnlock(t *testing.T) {
} }
// Then test force unlock of a lock that does not exist (not returning an error) // Then test force unlock of a lock that does not exist (not returning an error)
laForce.dsyncLockArgs.UID = "" laForce.LockArgs.UID = ""
laForce.SetRequestTime(time.Now().UTC()) laForce.SetRequestTime(time.Now().UTC())
err = locker.ForceUnlock(&laForce, &result) err = locker.ForceUnlock(&laForce, &result)
if err != nil { if err != nil {

@ -103,9 +103,9 @@ type LoginRPCReply struct {
// LockArgs represents arguments for any authenticated lock RPC call. // LockArgs represents arguments for any authenticated lock RPC call.
type LockArgs struct { type LockArgs struct {
AuthRPCArgs AuthRPCArgs
dsyncLockArgs dsync.LockArgs LockArgs dsync.LockArgs
} }
func newLockArgs(args dsync.LockArgs) LockArgs { func newLockArgs(args dsync.LockArgs) LockArgs {
return LockArgs{dsyncLockArgs: args} return LockArgs{LockArgs: args}
} }

Loading…
Cancel
Save