Skip to content

Upgrade pipelines with worker job#574

Closed
subdavis wants to merge 1 commit into
mainfrom
server/upgrade-pipelines
Closed

Upgrade pipelines with worker job#574
subdavis wants to merge 1 commit into
mainfrom
server/upgrade-pipelines

Conversation

@subdavis
Copy link
Copy Markdown
Contributor

@subdavis subdavis commented Feb 15, 2021

@BryonLewis what do you think about something like this?

Naturally the lack of worker management means other jobs couldn't be allowed to run while this was running, or you'd get pipe missing errors. I don't think stalling the queue using celery controller is reliable.

I think just adding a dive.pipelines_stalled bool setting while the job runs such that trying to start a pipeline while it's true throws a 409 conflict, and then requiring an empty queue and idle runners as a pre-condition to starting the upgrade.

It's sort of a coarse lock, but I think it's easier to understand than trying to drain the queue and stall jobs using celery controller.

@BryonLewis
Copy link
Copy Markdown
Collaborator

I think this would work immediately at least to ease the building process. Is the idea eventually that this endpoint is modified or a tool is put in place to specify the updates externally instead of relying on the two statically defined internal shell scripts?

@subdavis
Copy link
Copy Markdown
Contributor Author

You make a good point. There's no real reason to offload this to a bash script. Might as well allow for more granular updates.

@BryonLewis
Copy link
Copy Markdown
Collaborator

I haven't really wrapped my head around exactly what I wanted, but this may overlap a bit with #557.

@subdavis
Copy link
Copy Markdown
Contributor Author

I've come up with a possible solution. It's been evading me all day.

I have a couple of goals here:

  • Be able to update pipelines with a job.
  • Don't bloat the docker image by packaging pipelines with the build
  • Keep track of what pipeline packages we have to avoid duplicate updates.

In addition, we've been doing some dumb things with pipelines from the start

  • Worker and server should be separable. It makes no sense to have to volume mount between them to expose info
  • It would be great to be able to more easily control what pipelines are available without having to muck around on disk with file names and such

I would like to instead have a Pipeline discovery worker job.

  • An admin launches the discovery job, which looks in the default location (builtin pipeines), a bind mounted location for addons, and then optionally updates the addons if they've changed.
  • It collects these pipelines into a configuration object that it POSTS back to a girder item just like the brand configuration item.

For now, the schema of that config can be the same as it is now, but it could easily change to be your schema described in #557.

One issue is that we need to keep downloaded addons in a separate directory from the viame install tree. It's a bad idea to mix a container image directory tree (ephemeral) with a persistent tree, so there will have to be 2 locations to look for pipes.

@BryonLewis
Copy link
Copy Markdown
Collaborator

I like the idea of the worker and server being separate and being able to change pipelines externally without having to relaunch.

There is one possible hiccup with separating pipelines into different folders and that is some have what I think are relative cascading dependencies on the common_ pipes. dependencies. We made need to check with Matt to see if these can be separated and properly maintain their imports (Imagine there is a way to do this) as well as a list of all of the common pipelines that should be stored in a central location (hopefully those are all the common_ pipes.
Example tracker_track_user_selections.pipe imports:

  • common_default_input_with_downsampler.pipe which imports:
    • common_default_input.pipe
  • common_short_term_tracker.pipe

@subdavis
Copy link
Copy Markdown
Contributor Author

Thanks for pointing that out. For the short term, we can continue using the default pipeline location. It means we can never rm -rf to get rid of old pipes while running (would require a container restart) but it's something.

Hopefully Matt can help us resolve this in the future.

@subdavis subdavis mentioned this pull request Feb 25, 2021
@subdavis subdavis mentioned this pull request Mar 5, 2021
@subdavis subdavis closed this Mar 5, 2021
@subdavis subdavis deleted the server/upgrade-pipelines branch March 5, 2021 02:59
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

Successfully merging this pull request may close these issues.

2 participants