Skip to content
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

CLI is not safe for multi-process use #239

Open
ericmillin opened this issue Mar 8, 2021 · 3 comments
Open

CLI is not safe for multi-process use #239

ericmillin opened this issue Mar 8, 2021 · 3 comments

Comments

@ericmillin
Copy link
Contributor

ericmillin commented Mar 8, 2021

Description:

Running the CLI in parallel frequently causes the config file to be overwritten with empty values. I instrumented the SDK and saw that sometimes when one process was writing, another process would read. Those reads returned 0bytes with no error. They then wrote empty configs back to config.json.

You can see it happening here.

     0B 1615140454645170000-write-beg-34920    <<< PID 34920 write begins
     0B 1615140454645773000-read-beg-34924     <<< PID 34924 read begins
     0B 1615140454646638000-read-end-34924     <<< PID 34924 read ends (0b)
   3.9K 1615140454646771000-write-end-34920    <<< PID 34920 write ends (3.9k)
     0B 1615140454646951000-write-beg-34924    <<< PID 34924 write begins
   945B 1615140454647265000-write-end-34924    <<< PID 34924 write ends (945b)
     0B 1615140454746809000-read-beg-34924     <<< PID 34924 read begins
   945B 1615140454747004000-read-end-34924     <<< PID 34924 read ends (945b)

Here's what happens in order:

  1. proc A starts to write 3.9k bytes
  2. proc B reads 0 bytes
  3. proc A complete write of 3.9k bytes
  4. proc B writes 945 bytes
  5. proc B reads 945 bytes

The 945 byte write is for an empty config struct, effectively logging out all the other processes.

Impact:

The log outs make test automation painful. E.g., they are preventing us from running a large test suite in parallel. I expect users will see more of these errors because IBM Cloud has shortened the TTL of its oauth tokens. That requires plugins to refresh and write oauth tokens more often, which will lead to more conflicts.

This is also a problem when updating plugins.

Repro:

Create a simple plugin that calls RefreshIAMToken on start. Then run the code using a script similar to

#!/bin/bash

set -ex
COUNT=10

for i in `seq 1 $COUNT` ; do
  (
  while ibmcloud myplugin some-command; do
    true
  done
  ) &
done
wait
@duglin
Copy link

duglin commented Mar 8, 2021

Yep - I just started to see this last week in our automated testing in which we run 5 tests in parallel all from the same machine. As it stands today this testing is broken and we can't use it - so this is kind of a big issue for us.

@JohnStarich
Copy link

We've also seen these issues in the kubernetes-service plugin. Thanks @ericmillin for opening.

@weieigao Could we add some form of atomic updates to config files, no matter which process is writing or reading?

One easy way to do this, but loses a little consistency, is writing a whole new config file to a temporary file, then renaming it (mv) on top of the intended file. In Bash, that might look like this:

tmp=$(mktemp)
echo hello > "$tmp"
mv "$tmp" ~/myconfig.json

Where this could be run multiple times in parallel, such that each config update results in a valid config file.

@ericmillin
Copy link
Contributor Author

Want to clarify that this occurs even when all processes are using the same login session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants