fsxwindowsfileserver: fixing task concurrency issues #2690
Conversation
singholt
force-pushed
the
dev
branch
2 times, most recently
from
7c961f5
to
1d1bd71
Compare
| @@ -519,9 +515,34 @@ func (fv *FSxWindowsFileServerResource) retrieveFileSystemDNSName(fileSystemId s | |||
| return nil | |||
| } | |||
|
|
|||
| var mountLock sync.Mutex | |||
There was a problem hiding this comment.
Why do you need an additional lock while performing hostMount? Please add a comment to explain the rationale. Also, why is this outside the resource encapsulation?
There was a problem hiding this comment.
I'll add a comment for this
| var execCommand = exec.Command | ||
|
|
||
| func (fv *FSxWindowsFileServerResource) performHostMount(remotePath string, localPathArg string, username string, password string) error { | ||
| func (fv *FSxWindowsFileServerResource) performHostMount(remotePath string, username string, password string) error { | ||
| mountLock.Lock() |
There was a problem hiding this comment.
can you add a comment here documenting why a package level lock is needed here? so that we don't lose context on why this was added
There was a problem hiding this comment.
does this delay the resource creation for fsx tasks?
There was a problem hiding this comment.
I ran 10 tasks together and noticed about half a second delay between tasks transitioning into running state.
There was a problem hiding this comment.
I'll add a comment to add some context
singholt
force-pushed
the
dev
branch
2 times, most recently
from
c202012
to
16c0b65
Compare
Summary
This change fixes task concurrency issues with drive letter assignment during
fsxwindowsfileservertask resource initialization.Implementation details
performHostMountmethod.performHostMount()method.Testing
Launched 10 tasks concurrently and each of them gets its own drive letter now.
Description for the changelog
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.