imagej / imagej-updater

The automatic updater for ImageJ
Other
12 stars 15 forks source link

Prevent pointless server connections in headless mode #80

Closed frauzufall closed 4 years ago

frauzufall commented 4 years ago

We have to investigate how to deal with the updater routine in headless mode. As far as I know, headless updating is executed via the CommandLine class. Therefore it might be best to just stop the usual update check in headless mode. This is implemented in this PR and still needs to be tested.

ctrueden commented 4 years ago

We may also want to teach the updater to check only X times per day, for some reasonable value of X, even in GUI mode. This is because some cluster jobs run with xvfb, due to limitations in what you can with ImageJ1 in headless mode. I will take a crack at it now.

ctrueden commented 4 years ago
  1. I think we should do the headless check differently. I pushed a fixup commit showing what I mean. We can avoid running the CheckForUpdates command at all, by not invoking it when the shown UI is the headless one.

  2. Limiting the update check to X times per day is going to be a little tricky. Already, the updater stores a "latest nag" value and only checks again if that value is >24 hours old. So one of two things is happening with these cluster jobs: either A) each execution of Fiji happens in a different sandbox and so the latest nag is not persisted across that boundary; or B) the latest nag is not being persisted at all. Looking at the UpToDate.java code, the latest nag appears to be cleared whenever an UP_TO_DATE result is given, meaning that if these jobs are running with fully updated Fijis, it will check every single time they launch.

I guess for now, the headless check will be a huge improvement. I'm still concerned about the xvfb case though.

ctrueden commented 4 years ago

OK, I pushed another commit that I hope will resolve the xvfb issue. It won't fix issue (A) above, though: if every Fiji launch happens in its own sandbox, then those launches will still each check for updates. But we can mitigate on the server side as well by imposing temporary IP blocking.

ctrueden commented 4 years ago

I also added a property imagej.updater.disableAutocheck which when set to true also skips the auto-check. We can add this to the documentation of https://imagej.net/Headless and elsewhere.

frauzufall commented 4 years ago

Looking at the UpToDate.java code, the latest nag appears to be cleared whenever an UP_TO_DATE result is given, meaning that if these jobs are running with fully updated Fijis, it will check every single time they launch.

I also had a closer look and the latest nag is only used to delay asking for updates in case the user clicks "Ask me later" when there is an update available. So it makes sense to clear it once Fiji is up to date. It's not really meant to make Fiji by default only update once per day.

frauzufall commented 4 years ago

Sorry, I lost track. I would do a rebase for the fixup and merge if there are no more improvements we could add @ctrueden?

ctrueden commented 4 years ago

Thanks!

karlduderstadt commented 1 year ago

I also added a property imagej.updater.disableAutocheck which when set to true also skips the auto-check. We can add this to the documentation of https://imagej.net/Headless and elsewhere.

@ctrueden Is there an easily way to set this property in headless mode from the command line?

I am creating a singularity container and I would like to set this during container creation so that the Fiji in the container never tries to update when running. It would always be run in headless mode.

If you can tell me a simple way to set this, I would be happy to add the information to the wiki page you mentioned.

ctrueden commented 1 year ago

@karlduderstadt Yep, you should be able to pass -Dimagej.updater.disableAutocheck=true as an argument. You might need to feed it as a JVM parameter rather than a SciJava-side parameter, by using the double-dash:

./ImageJ-linux64 -Dimagej.updater.disableAutocheck=true -- --headless --ij2 --run org.scijava.plugins.commands.debug.SystemInformation

In the case of -D parameters, there is actually also a Java-side ConsoleArgument handler for them, but in the case of the update check flag, I don't recall offhand whether this will end up setting the property early enough. You could try it without the -- and see what happens...

If this works for you (one way or another), please feel free to file a PR against the https://imagej.net/scripting/headless page explaining it!

karlduderstadt commented 1 year ago

Thanks a lot @ctrueden! This works perfectly and all the other information reported by org.scijava.plugins.commands.debug.SystemInformation is extremely helpful. I didn't know about that command.

One thing I noticed is that, unlike preferences, the imagej.updater.disableAutocheck=true setting does not seem to persist between launches, so I guess this parameter has to be added every time. Is that correct?

This is not a problem, just an observation that I need to make sure I add this in the runscript section of my singularity container every time I launch Fiji headless. Once this is confirmed, I will add it to the page with an explanation.

In my case, I am really using it headless without xvfb, so maybe the check wouldn't happen anyway because of the line below, but I will add the property and update the docs page in any case. https://github.com/imagej/imagej-updater/blob/dd4b18af362bd46faae8820bb17d227817ad71f4/src/main/java/net/imagej/updater/DefaultUpdateService.java#L115

ctrueden commented 1 year ago

One thing I noticed is that, unlike preferences, the imagej.updater.disableAutocheck=true setting does not seem to persist between launches, so I guess this parameter has to be added every time. Is that correct?

That is correct. It is a system property, which overrides the Updater logic to be skipped. There is no user-facing persistent preference to completely skip update checks.

maybe the check wouldn't happen anyway because of the line below

Yeah, I am guessing it won't check for updates while headless, even without the disableAutocheck flag being set.

imagesc-bot commented 1 year ago

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/singularity-container-for-fji/23311/9

karlduderstadt commented 1 year ago

Thanks @ctrueden,

I added a couple lines of usage instructions to prevent the updater from running on https://imagej.net/scripting/headless. I was able to directly push the changes since I made some corrections in the past. I will check once it is live that everything looks correct.