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

The Great Decoupling #3307

Merged
merged 21 commits into from Mar 15, 2019
Merged

The Great Decoupling #3307

merged 21 commits into from Mar 15, 2019

Conversation

@pivotal-jwinters
Copy link
Contributor

pivotal-jwinters commented Feb 14, 2019

This PR is based off #3225 (which fixes #2826)

It also fixes #3240

...and fixes #3267

...and fixes #2895

Copy link
Member

vito left a comment

Left a bunch of inline comments. Looks good though, excited for this! 👍

Here are the main points:

  • We'll probably need to tweak the build plan -> schema migration so it doesn't hold all the data in its head at once.
  • fly execute no longer has upload/download progress; it'd be nice to bring that back.
  • I'm iffy on the volume lifecycle:
    • I think we should figure out the race condition for the user-created artifacts.
    • It seems like leaving the artifacts around for 12 hours could lead to piling up disk usage as pipelines run.
  • I think we should still strive for "fast" - we talked about using 'LISTEN'/'NOTIFY' for this in IPM. I think we'd need one to short-circuit build tracking and one to short-circuit pipeline scheduling.
  • Not sure about creating builds in 'started' state. Maybe we need a general 'build starter' similar to how we have a 'build tracker'? (Maybe they're the same thing?) If we had that, could we change the pipeline scheduler so that it just inserts pending builds and leaves the rest up to the starter/tracker?
  • Looks like manually-triggered builds should be marked as manually triggered somehow, to make sure we uphold the contract of performing 'check's prior to determining their inputs.
atc/api/buildserver/create.go Show resolved Hide resolved
atc/api/accessor/accessor_test.go Outdated Show resolved Hide resolved
atc/api/artifactserver/create.go Show resolved Hide resolved
atc/api/artifactserver/create.go Show resolved Hide resolved
atc/exec/task_step.go Outdated Show resolved Hide resolved
atc/plan.go Show resolved Hide resolved
atc/scheduler/runner.go Show resolved Hide resolved
atc/worker_artifact.go Show resolved Hide resolved
@pivotal-jwinters

This comment has been minimized.

Copy link
Contributor Author

pivotal-jwinters commented Mar 12, 2019

  • batch migration
  • get artifact -> member only
  • creating volume
  • upload/download status
  • artifact lifecycle
  • double status
@vito vito mentioned this pull request Mar 12, 2019
pivotal-jwinters added a commit that referenced this pull request Mar 13, 2019
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
pivotal-jwinters added a commit that referenced this pull request Mar 13, 2019
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
pivotal-jwinters added a commit that referenced this pull request Mar 13, 2019
* instead of for every task

#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
@pivotal-jwinters

This comment has been minimized.

Copy link
Contributor Author

pivotal-jwinters commented Mar 13, 2019

@vito based on these commits I just pushed, and some comments, I think I addressed all your concerns. Let me know if you think there's something that I missed.

pivotal-jwinters added a commit that referenced this pull request Mar 13, 2019
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
@pivotal-jwinters

This comment has been minimized.

Copy link
Contributor Author

pivotal-jwinters commented Mar 13, 2019

Also fixed the double status. NOW it should be good to go.

@pivotal-jwinters pivotal-jwinters force-pushed the feature/3240-the-great-decoupling branch from f3a3e71 to f629101 Mar 13, 2019
pivotal-jwinters added a commit that referenced this pull request Mar 13, 2019
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
pivotal-jwinters added a commit that referenced this pull request Mar 13, 2019
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
pivotal-jwinters added a commit that referenced this pull request Mar 13, 2019
* instead of for every task

#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
pivotal-jwinters added a commit that referenced this pull request Mar 13, 2019
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
@pivotal-jwinters pivotal-jwinters force-pushed the feature/3240-the-great-decoupling branch from f629101 to 1e714af Mar 14, 2019
pivotal-jwinters added a commit that referenced this pull request Mar 14, 2019
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
pivotal-jwinters added a commit that referenced this pull request Mar 14, 2019
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
pivotal-jwinters added a commit that referenced this pull request Mar 14, 2019
* instead of for every task

#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
pivotal-jwinters added a commit that referenced this pull request Mar 14, 2019
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
pivotal-jwinters and others added 5 commits Dec 19, 2018
* the handler uses a worker client to create a volume
* it then streams the payload of the request into the volume
* currently there is no way to delete a user artifact, but
if the worker_artifact record goes away the volume gets gc'ed
* we can now use these endpoints for fly execute

#2826

Signed-off-by: Josh Winters <jwinters@pivotal.io>
Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Topher Bullock <cbullock@pivotal.io>
Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Topher Bullock <cbullock@pivotal.io>
Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Topher Bullock <cbullock@pivotal.io>
Signed-off-by: Josh Winters <jwinters@pivotal.io>
pivotal-jwinters and others added 15 commits Jan 14, 2019
Co-authored-by: Krishna Mannem <kmannem@pivotal.io>
Signed-off-by: Josh Winters <jwinters@pivotal.io>
instead of a calling enginebuild.Resume in the handler

*wip note*

we are currently hardcoding the engine as 'exec.v2' when creating
started builds in the databse. We should probably leave this up to
the dbengine to update the engine field if its empty.

we should also probably change the engine_metadata to private_plan
and reintroduce a metadata field later if needed.

#3240

Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Mark Huang <mhuang@pivotal.io>
* it no longer relies on the exec engine

#3240

Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Mark Huang <mhuang@pivotal.io>
* it gets run during the next scheduler tick

#3240

Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Mark Huang <mhuang@pivotal.io>
* this was a weird side effect of a GET request

#3240

Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Mark Huang <mhuang@pivotal.io>
* engine -> schema
* engine_metadata -> private_plan

#3240

Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Mark Huang <mhuang@pivotal.io>
* remove all the scheduler/engine deps from the api

* also remove TriggerImmediately and SaveNextInputMappings
from the scheduler

#3240

Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Mark Huang <mhuang@pivotal.io>
* because we don't need it anymore

#3267

Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Mark Huang <mhuang@pivotal.io>
#3267

Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Mark Huang <mhuang@pivotal.io>
Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Mark Huang <mhuang@pivotal.io>
Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Mark Huang <mhuang@pivotal.io>
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
* instead of for every task

#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
#3307

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
@pivotal-jwinters pivotal-jwinters force-pushed the feature/3240-the-great-decoupling branch from 1e714af to 6828e5c Mar 14, 2019
Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
@vito
vito approved these changes Mar 15, 2019
@vito vito merged commit ed58772 into master Mar 15, 2019
5 checks passed
5 checks passed
DCO DCO
Details
WIP Ready for review
Details
concourse-ci/testflight Concourse CI build success
Details
concourse-ci/unit Concourse CI build success
Details
concourse-ci/watsjs Concourse CI build success
Details
@vito vito added this to the v5.1.0 milestone Apr 5, 2019
@vito vito deleted the feature/3240-the-great-decoupling branch Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.