search

IBM / ubiquity

Ubiquity
Apache License 2.0
90 stars 26 forks source link

Fix/increase locks sleep and run discover only in mount flow (not in unmount flow) #167

Closed shay-berman closed 6 years ago

shay-berman commented 6 years ago

  1. increase sleep time on mpath lock and rescan lock from 100 to 500ms, so it will have less read IO while waiting for lock (since rescan usually take 1-2 seconds, so no need to have so many retries)

  2. Fix wrong debug message in rescanLock (by mistake it was printed as mpathLock). Found by @danapython

  3. Do not run discover during rescan if the rescan was triggered during detach. becuase during detach the discover will always fail and its a waist of time and also it will start run sg_inq on each device which is also waist of time and few error in the log that are not relevant. SO move the discover during rescan only if rescan triggered for attach. this is help us also to identify when we really have bad WWN show up in the multipath -ll.

  4. Change message log from Debug to Error if discover doesn't find the WWN in multipath (usually it indicate the bad multipathing WWN in multipath -ll output. so its better to be in error level rather then debug level)

This change is Reviewable

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.04%) to 54.81% when pulling 98602a23ffa4fd5952aaa76b72c62273d048aed4 on fix/increace_locks_sleep_and_run_discover_only_if_in_mount into 2cd22430651a60aaf4eb4076e73569d6696bfe85 on dev.

alonm commented 6 years ago

remote/mounter/block_device_mounter_utils/block_device_mounter_utils.go, line 97 at r1 (raw file):

      }
      b.logger.Debug("mpathFlock.TryLock failed", logs.Args{{"error", err}})
      time.Sleep(time.Duration(500*time.Millisecond))

better make it a constant instead of hard-code value

Comments from Reviewable

alonm commented 6 years ago

remote/mounter/block_device_mounter_utils/block_device_mounter_utils.go, line 134 at r1 (raw file):


  if !rescanForCleanUp {
      // Only when run rescan for new device, try to check if its already exist to reduce rescans

"Check if device exists only for new device"

Comments from Reviewable

ranhrl commented 6 years ago
:lgtm:

Reviewed 2 of 2 files at r1. Review status: all files reviewed at latest revision, 2 unresolved discussions.

Comments from Reviewable

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.04%) to 54.81% when pulling e4f386097eadbc7165fb6423f38d95eec3e58da7 on fix/increace_locks_sleep_and_run_discover_only_if_in_mount into 2cd22430651a60aaf4eb4076e73569d6696bfe85 on dev.