Navigation Menu

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

List volumes endpoint errors if any volume disappears while presenting #1473

Closed
vito opened this issue Aug 15, 2017 · 4 comments
Closed

List volumes endpoint errors if any volume disappears while presenting #1473

vito opened this issue Aug 15, 2017 · 4 comments

Comments

@vito
Copy link
Member

vito commented Aug 15, 2017

We currently make 3 DB calls per volume, making this endpoint incredibly slow and error prone:

https://github.com/concourse/atc/blob/master/api/present/volume.go#L9-L23

@jtarchie
Copy link
Contributor

We are going to make this a single query, not an N+1.

@jtarchie
Copy link
Contributor

We saw that when any of volumes errored (ie: something got deleted) the entire endpoint failed. We've fixed it up so that it only returns volumes that haven't errored, logs the errored volumes, and get's you ice cream (not really).

We also took a crack, eh, at unifying the query for the endpoint, so that we wouldn't run into this issue. We have a single query that can return the necessary information. The inclusion of the query would change quite a bit of modeling outside this endpoint -- mainly db/volume.go.

If we want to move on with a remodeling, let's discuss and schedule some work.

And my pair will copy+pasta a nice SQL statement.

@jwntrs
Copy link
Contributor

jwntrs commented Nov 30, 2017

WITH RECURSIVE
worker_base_resource_type_version AS (
	SELECT wbrt.id wbrt_id, brt.id brt_id, brt.name, wbrt.worker_name, wbrt.version
	FROM worker_base_resource_types wbrt
	LEFT JOIN base_resource_types brt ON brt.id = wbrt.base_resource_type_id
), 
base_resource_type_configs AS (
		SELECT rc.id, rcfg.base_resource_type_id, rcfg.resource_cache_id
		FROM resource_caches rc, resource_configs rcfg 
		WHERE rcfg.id = rc.resource_config_id
),
base_resource_configs(id, base_resource_type_id, resource_cache_id) AS (
	SELECT * FROM base_resource_type_configs brtc 
		WHERE brtc.base_resource_type_id IS NOT NULL
	UNION ALL
	SELECT c.id, p.base_resource_type_id, c.resource_cache_id
		FROM base_resource_configs AS p, base_resource_type_configs c
		WHERE p.id = c.resource_cache_id
)
SELECT
	v.id,
	v.handle,
	v.state,
	w.name,
	v.path,
	c.handle,
	pv.handle,
	v.team_id,
	wrc.resource_cache_id,
	v.worker_base_resource_type_id,
	v.worker_task_cache_id,
	rc.version,
	wbrtv_resource.name resource_type_base_resource_type_name,
	wbrtv_resource.version resource_type_base_resource_type_version,
	wbrtv_base_resource.name base_resource_type_name,
	wbrtv_base_resource.version base_resource_type_version,
	p.name,
	j.name,
	wtc.step_name,
	CASE
		WHEN v.worker_base_resource_type_id IS NOT NULL THEN 'resource-type'
		WHEN v.worker_resource_cache_id IS NOT NULL THEN 'resource'
		WHEN v.container_id IS NOT NULL THEN 'container'
		WHEN v.worker_task_cache_id IS NOT NULL THEN 'task-cache'
		ELSE 'unknown'
	END
FROM volumes v
LEFT JOIN workers w ON v.worker_name = w.name
LEFT JOIN containers c ON v.container_id = c.id
LEFT JOIN volumes pv ON v.parent_id = pv.id
LEFT JOIN worker_resource_caches wrc ON wrc.id = v.worker_resource_cache_id
LEFT JOIN resource_caches rc ON rc.id = wrc.resource_cache_id
LEFT JOIN base_resource_configs brc ON brc.id = rc.id
LEFT JOIN worker_base_resource_type_version wbrtv_resource ON wbrtv_resource.brt_id = brc.base_resource_type_id AND wbrtv_resource.worker_name = w.name
LEFT JOIN worker_base_resource_type_version wbrtv_base_resource ON wbrtv_base_resource.wbrt_id = v.worker_base_resource_type_id
LEFT JOIN worker_task_caches wtc ON wtc.id = v.worker_task_cache_id
LEFT JOIN jobs j ON j.id = wtc.job_id
LEFT JOIN pipelines p ON p.id = j.pipeline_id
WHERE (v.team_id = $1 OR v.team_id IS NULL);

jwntrs pushed a commit to vmware-archive/atc that referenced this issue Nov 30, 2017
concourse/concourse#1473

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
jwntrs pushed a commit that referenced this issue Nov 30, 2017
#1473

Submodule src/github.com/concourse/atc ae0bbc6..56aa730d:
  > don't fail when one volume errors on /volumes
  > run go fmt

Signed-off-by: Joshua Winters <jwinters@pivotal.io>
@vito
Copy link
Member Author

vito commented Jan 10, 2018

I've split out #1947 for the optimization aspect, so I'll just do acceptance by seeing that it doesn't blow up anymore. The code change looks good.

@vito vito added the accepted label Jan 15, 2018
@vito vito closed this as completed Jan 15, 2018
@vito vito modified the milestone: v3.9.0 Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants