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

Randomly select Relay at beginning of stage only #790

Closed
wants to merge 3 commits into from

Conversation

kevsmith
Copy link
Member

This commit modifies the Relay selection logic in Cog.Executor such that a Relay is randomly selected once at the start of each pipeline stage and is then reused for subsequent invocations for that stage. Doing so allows Cog to take maximum advantage of Relay's ability to reuse containers for the same pipeline stage and is expected to improve performance for Cog installations with multiple Relays.

This commit modifies the Relay selection logic in Cog.Executor such that
a Relay is randomly selected once at the start of each pipeline stage
and is then reused for subsequent invocations for that stage. Doing so
allows Cog to take maximum advantage of Relay's ability to reuse
containers for the same pipeline stage and is expected to improve
performance for Cog installations with multiple Relays.
@mpeck
Copy link
Contributor

mpeck commented Jun 22, 2016

👍

This looks reasonable to me, but I do have a question. If you are in the middle of a long running plan, and the bundle is unassigned from the relay, what happens? Would Cog just crash?

@kevsmith
Copy link
Member Author

kevsmith commented Jun 22, 2016

The container wouldn't get torn down when the bundle is removed. Container destruction happens once it's idle. In the scenario you describe I'd expect the plan for the pipeline stage to finish without error (assuming the command invocations didn't throw any) but future invocations wouldn't select the same Relay(s).

UPDATE: What I said isn't entirely accurate. The Docker environment is unreachable once the bundle has been removed from the Relay. A more accurate description of behavior is that the Relay would continue to service invocations until the bundle has been removed. When that happens it will return a "bundle not found" error back to Cog. So, yes, the pipeline will crash if that happens before the invocations have finished.

@christophermaier
Copy link
Collaborator

This isn't going to do what we want; an invocation can generate multiple plans, each of which is completely isolated from the others in its cohort.

We're going to need to remember the relay that's chosen at a higher level, probably in the executor state itself. I had a proposal for this in #777.

@mpeck
Copy link
Contributor

mpeck commented Jun 22, 2016

@kevsmith cool that makes sense. I asked more out of curiosity than anything, but is that something we should actually worry about? Should we check to see if relay is still valid on each execution and pick a new relay if it isn't?

Previous iteration assigned Relays when each plan was executed. This was
wrong as a single invocation can generate multiple plans which negated
the benefits of selecting a Relay when each plan executes.

This commit moves Relay selection into the planner. A Relay is selected
before plans for an invocation are created and shared across all the
created plans.
@kevsmith kevsmith closed this Jun 22, 2016
@kevsmith kevsmith removed the review label Jun 22, 2016
@kevsmith kevsmith deleted the kevsmith/cache-invoked branch June 22, 2016 16:57
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.

3 participants