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

Added ability to inject delay #43

Closed
wants to merge 1 commit into from

Conversation

owenfarrell
Copy link
Member

As a developer, I want the ability to inject a fixed delay in to my build plan so that I can account for long-running operations that cannot easily be accessed via Concourse (e.g. replication, provisioning).

Signed-off-by: Owen Farrell <ofarrell@captechconsulting.com>
Copy link
Member

@vito vito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting this! I think it's an interesting idea, since it'll let people avoid having to use a task just for sleeping - though that may make more sense once concourse/rfcs#7 is figured out.

@@ -63,17 +63,19 @@ destination.

#### Parameters

*None.*
* `wait`: *Optional.* If `true`, pauses execution until the given timestamp.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Can you show how this would be used? It looks interesting, and I think I have a guess:

put: delay
params: {after: 1h}
get_params: {wait: true}

So the idea is to use the put to create a new timestamp and then immediately wait for it to elapse in the subsequent get step?

This seems like an interesting way to frame it, but I wonder if it would be simpler to just have the sleep occur in the put?

The model you've proposed will currently allow you to put in one job without waiting and then get + wait in downstream jobs instead, but that relies on behavior we're planning to change: concourse/concourse#3463 will only collect versions from check. put steps can still create arbitrary versions, they just won't be recorded in the version history, and thus won't be able to propagate to downstream jobs.

Copy link
Member Author

@owenfarrell owenfarrell Aug 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The model you've proposed will currently allow you to put in one job without waiting and then get + wait in downstream jobs instead

That was exactly what I was going for here. Staggering the creation of a timestamp (out) and the retrieval of that timestamp (in) seemed like the most efficient way to achieve maximum flexibility. But maybe I'm interpreting the spirit of the Concourse resource contract too literally.

but that relies on behavior we're planning to change

Well that's a bummer. Forgive me if I ask a stupid question here, but I'm trying to wrap my head around the implications of that change.

There are a handful of scenarios where the first reference to a resource version is its creation via a pipeline: semver atomic puts, creating a file in s3, creating GitHub release. How do you plan to handle those scenarios?

For example:

resources:
- name: 5m
  type: time
  source: { interval: 5m }

jobs:
- name: sample_put
  plan:
  - put: 5m
    params: { after: 12m }

- name: sample_get
  plan:
  - get: 5m
    passed: [sample_put] // What version gets pulled here?
    params: { wait: true }
    trigger: true

If the "to be" implementation will block a fan-in until the put version (or a subsequent version) is returned from check, then I think this implementation would still work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@owenfarrell Having the put be the first reference to it is fine, it just has to be a version detectable by check. For "proper" resource implementations whose get/put/check are all complementary and have an external source of truth, everything should just continue to work, albeit with a slight delay as the check polling interval elapses. (We could also speed that up by queueing up a check now that we have a check queue: concourse/concourse#3788.)

This restriction will affect resources whose put produces totally unique versions that cannot be discovered by check - in this case, the time resource would be affected and your proposed change would only be usable within a single job.

So with that in mind I'm thinking this might make more sense as a re-usable task, rather than a resource action. I would model your approach by just passing your inputs through an intermediate job that just runs a sleep.

One advantage of this is it clarifies the job state a bit; having the downstream jobs all wait at the beginning makes it hard know at a glance whether they're actually running or just waiting. Moving the sleep into a separate job makes that clearer. It's also one place to abort if you want to halt the pipeline in the middle of the delay.

All that being said, we don't really have a great model for reusable tasks just yet, but something as simple as sleep should be fairly straightforward. There is some precedent for re-usable tasks in https://github.com/concourse/builder-task - we're taking our learnings from it and will address them with concourse/rfcs#7 once we have bandwidth to do so.

Would that approach work for you? 🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That approach would toally work. I think the downside is the cost of delay - the longer it takes for reusable tasks to be implemented, the more hacks/workarounds will be out in the wild in the meantime. But I don't know that should be taken in to consideration.

For "proper" resource implementations whose get/put/check are all complementary and have an external source of truth, everything should just continue to work, albeit with a slight delay as the check polling interval elapses.

If I pull that thread a little bit further - doesn't that mean that this resource needs to be updated to be more proper? To put it differently, should we update out to round up/down to the nearest version that will be returned by check?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this resource would be affected. The put from this resource would just give you a timestamp for that build but not record it in the history, so it couldn't be used for propagating versions.

Reusable tasks are already a thing - the builder task is one example which seems to be working pretty well. The concourse/rfcs#7 proposal is more around putting a bow on it and making it feel less janky. For something as simple as sleep I think there could easily just be a sleep task that someone maintains.

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