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

Salmon 1.1.0 auto update check stalled #486

Closed
cihanerkut opened this issue Feb 24, 2020 · 12 comments
Closed

Salmon 1.1.0 auto update check stalled #486

cihanerkut opened this issue Feb 24, 2020 · 12 comments

Comments

@cihanerkut
Copy link

salmon command gives as expected:

salmon v1.1.0

Usage:  salmon -h|--help or
        salmon -v|--version or
        salmon -c|--cite or
        salmon [--no-version-check] <COMMAND> [-h | options]

Commands:
     index Create a salmon index
     quant Quantify a sample
     alevin single cell analysis
     swim  Perform super-secret operation
     quantmerge Merge multiple quantifications into a single file

However, salmon index --help freezes without any CPU usage. strace shows it's trying to connect to an IP address, which belongs to GitHub but out of commission right now.

socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 6
epoll_ctl(4, EPOLL_CTL_ADD, 6, {EPOLLIN|EPOLLPRI|EPOLLERR|EPOLLHUP|EPOLLET, {u32=509961152, u64=140196832437184}}) = 0
ioctl(6, FIONBIO, [1])                  = 0
connect(6, {sa_family=AF_INET, sin_port=htons(80), sin_addr=inet_addr("185.199.108.153")}, 16) = -1 EINPROGRESS (Operation now in progress)
epoll_ctl(4, EPOLL_CTL_MOD, 6, {EPOLLIN|EPOLLPRI|EPOLLOUT|EPOLLERR|EPOLLHUP|EPOLLET, {u32=509961152, u64=140196832437184}}) = 0
epoll_wait(4, ^Cstrace: Process 4488 detached

Waiting long enough, it tries another IP address, which is also broken.

My Salmon is compiled with GCC 8.2.0 / gompi2019a toolset using EasyBuild. It runs on Centos 7.

Passing --no-version-check solves the problem but this option isn't displayed on main help screen, I found it somewhere in the source.

@cihanerkut
Copy link
Author

cihanerkut commented Feb 25, 2020

The same with version 1.2.0.

Is there a way to disable version check by default, completely? I install Salmon as a module and I wouldn't let it update itself automatically, anyway.

@rob-p
Copy link
Collaborator

rob-p commented Feb 27, 2020

Hi @cihanerkut,

First, thanks for reporting this, and going through the trouble to give the strace information. Second, salmon won't auto-update anyway. That would be quite slick, but there's not a good and reliable way to do that with natively-compiled programs that I know of.

However, the most surprising thing is that you are finding the call to the version check ip to be hanging for any significant amount of time. The timeout should be pretty quick. How long does it hang when you do salmon index --help? I'll look into it on our end as well.

@cihanerkut
Copy link
Author

Hi @rob-p,

The first attempt timed out in 2 min 7 sec.

22:06:42 socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 6
22:06:42 epoll_ctl(4, EPOLL_CTL_ADD, 6, {EPOLLIN|EPOLLPRI|EPOLLERR|EPOLLHUP|EPOLLET, {u32=497378240, u64=139780208026560}}) = 0
22:06:42 ioctl(6, FIONBIO, [1])         = 0
22:06:42 connect(6, {sa_family=AF_INET, sin_port=htons(80), sin_addr=inet_addr("185.199.109.153")}, 16) = -1 EINPROGRESS (Operation now in progress)
22:06:42 epoll_ctl(4, EPOLL_CTL_MOD, 6, {EPOLLIN|EPOLLPRI|EPOLLOUT|EPOLLERR|EPOLLHUP|EPOLLET, {u32=497378240, u64=139780208026560}}) = 0
22:06:42 epoll_wait(4, [{EPOLLIN|EPOLLOUT|EPOLLERR|EPOLLHUP, {u32=497378240, u64=139780208026560}}], 128, -1) = 1
22:08:49 poll([{fd=6, events=POLLOUT}], 1, 0) = 1 ([{fd=6, revents=POLLOUT|POLLERR|POLLHUP}])
22:08:49 getsockopt(6, SOL_SOCKET, SO_ERROR, [110], [4]) = 0
22:08:49 close(6)                       = 0

@EricDeveaud
Copy link

The same with version 1.2.0.

Is there a way to disable version check by default, completely? I install Salmon as a module and I wouldn't let it update itself automatically, anyway.

--- Salmon.cpp.ori  2020-04-21 15:12:29.916219870 +0000
+++ Salmon.cpp  2020-04-21 15:16:48.488926415 +0000
@@ -53,7 +53,7 @@
       "Usage:  salmon -h|--help or \n"
       "        salmon -v|--version or \n"
       "        salmon -c|--cite or \n"
-      "        salmon [--no-version-check] <COMMAND> [-h | options]\n\n");
+      "        salmon <COMMAND> [-h | options]\n\n");
   helpMsg.write("Commands:\n");
   helpMsg.write("     index Create a salmon index\n");
   helpMsg.write("     quant Quantify a sample\n");
@@ -171,8 +171,6 @@
     // https://gist.github.com/randomphrase/10801888
     po::options_description sfopts("Allowed Options");
     sfopts.add_options()("version,v", "print version string")(
-        "no-version-check",
-        "don't check with the server to see if this is the latest version")(
         "cite,c", "show citation information")(
         "help,h", "produce help message")("command", po::value<string>(),
                                           "command to run {index, quant, sf}")(
@@ -209,11 +207,6 @@
       std::exit(0);
     }

-    if (!vm.count("no-version-check")) {
-      std::string versionMessage = getVersionMessage();
-      std::cerr << versionMessage;
-    }
-
     // po::notify(vm);

     std::string cmd = vm["command"].as<std::string>();

@EricDeveaud
Copy link

I would have prefered an option to check if a new version is available instead of having a software that systematicly phone home

it will be problematic on our cluster as compute nodes does not have network acces to internet.
did not checked yet how this case in handled by salmon

regards

Eric

@rob-p
Copy link
Collaborator

rob-p commented Apr 21, 2020

@EricDeveaud,

Does passing --no-version-check resolve the issue? This flag goes before any other command and disables this behavior. For example:

salmon --no-version-check index -t <gentrome.fa> -d decoys.txt -i index

or

salmon --no-version-check quant -i index -la -1 reads_1.fq.gz -2 reads_2.fq.gz -o quant_dir

There should be no network access when using this flag.

@EricDeveaud
Copy link

yes --no-version-check does the trick, but among all the users of the cluster I'm pretty sure some of them will forgot ;-)

on our local installation I disabled the getVersionMessage even if salmon handle the no network cleanly. (I tested using unshare -n salmon whatever you want)
NB debian maintainer also disabled the phone home call in their packages

sorry if it it may sound harsh

@cihanerkut
Copy link
Author

Just an idea. Would it be possible to assign an environment variable, such as SALMON_NO_VERSION_CHECK, whose existence overrides version checking? This wouldn't break compatibility with older scripts because they wouldn't have the variable in the first place. In non-networked nodes, an admin can simply set this variable and users will run salmon as usual.

@rob-p
Copy link
Collaborator

rob-p commented Apr 21, 2020

@cihanerkut,

Yes; this is a nice solution and I think it wouldn't be too hard!

@rob-p
Copy link
Collaborator

rob-p commented Apr 22, 2020

@cihanerkut and @EricDeveaud,

We just released 1.2.1, which is on the release page, and dockerhub, and should propagate to bioconda soon. It adds support for the SALMON_NO_VERSION_CHECK environment variable. If you set SALMON_NO_VERSION_CHECK to either 1 or TRUE in the environment where salmon is running, it will skip the version check. I hope this helps!

--Rob

@rob-p rob-p closed this as completed Apr 22, 2020
@EricDeveaud
Copy link

thanks

@cihanerkut
Copy link
Author

Thank you very much!

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