Skip to content

Duplicate name when creating a new workspace build (transition): pq: duplicate key value violates unique constraint "workspace_builds_workspace_id_name_key" #3038

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

Closed
mafredri opened this issue Jul 19, 2022 · 4 comments
Labels
api Area: HTTP API

Comments

@mafredri
Copy link
Member

mafredri commented Jul 19, 2022

This is an issue we ran into CI, it's a rare occurrence but there's a non-zero chance it can happen to our users as well.

Problem: When a new workspace build is created for an existing workspace (e.g. transition into deleted) a new (unique-ish) name is generated for that job via namesgenerator.GetRandomName(1). Normally this should not be an issue, but we have a unique constraint on (workspace_id, name):

ALTER TABLE ONLY workspace_builds
    ADD CONSTRAINT workspace_builds_workspace_id_name_key UNIQUE (workspace_id, name);

This results in a non-zero chance that both the create job and delete job receive the same name, resulting in the conflict. For very long lived workspaces, the chance of hitting this issue becomes greater. I did a local test that reproduced this after running workspace start/stop 200 times.

The CI run that hit this issue: https://github.com/coder/coder/runs/7399324482?check_suite_focus=true

2022-07-18T22:52:32.7769558Z     workspaces_test.go:331: 
2022-07-18T22:52:32.7770135Z         	Error Trace:	/home/runner/work/coder/coder/coderd/workspaces_test.go:331
2022-07-18T22:52:32.7770615Z         	Error:      	Received unexpected error:
2022-07-18T22:52:32.7772201Z         	            	POST http://127.0.0.1:42617/api/v2/workspaces/9a172a87-b686-4a93-a286-70d473562375/builds: unexpected status code 500: Internal error inserting workspace build.
2022-07-18T22:52:32.7773646Z         	            		Error: execute transaction: insert workspace build: pq: duplicate key value violates unique constraint "workspace_builds_workspace_id_name_key"
2022-07-18T22:52:32.7774246Z         	Test:       	TestWorkspaceByOwnerAndName/Deleted
2022-07-18T22:52:32.7774730Z         	Messages:   	delete the workspace
2022-07-18T22:52:32.7775638Z     t.go:81: 2022-07-18 22:49:15.883 [DEBUG]	(provisionerd)	<github.com/coder/coder/provisionerd/provisionerd.go:370>	(*Server).closeWithError	closing server with error	{"error": null}
2022-07-18T22:52:32.7776288Z     --- FAIL: TestWorkspaceByOwnerAndName/Deleted (6.92s)
2022-07-18T22:52:32.7776505Z 
2022-07-18T22:52:32.7776777Z make: *** [Makefile:175: test-postgres] Error 1

Questions

  • Is there a good reason to keep unique names for all workspace builds (and all transitions)?
  • Should we try a new name on conflict?
@mafredri mafredri added bug api Area: HTTP API labels Jul 19, 2022
@mafredri
Copy link
Member Author

mafredri commented Jul 20, 2022

Another instance of this issue has manifested in https://github.com/coder/coder/runs/7429175788?check_suite_focus=true

    coderdtest.go:377: 
        	Error Trace:	/Users/runner/work/coder/coder/cli/coderdtest.go:377
        	            				/Users/runner/work/coder/coder/cli/templatedelete_test.go:60
        	Error:      	Received unexpected error:
        	            	POST http://127.0.0.1:49396/api/v2/organizations/b5a4944f-dcce-4e89-b1e6-c30d8f2b3ffa/templates: unexpected status code 409: Template with name "mystifying-rosalind" already exists.
        	            		name: This value is already in use and should be unique.
        	Test:       	TestTemplateDelete/Multiple_--yes
    t.go:81: 2022-07-20 12:33:35.980 [DEBUG]	(provisionerd)	<github.com/coder/coder/provisionerd/provisionerd.go:370>	(*Server).closeWithError	closing server with error	{"error": null}
    --- FAIL: TestTemplateDelete/Multiple_--yes (1.11s)

We should add the ability to retry unique names until we find one in all places where we rely on them or enforce uniqueness.

Edit: This looks like a test bug, originating from:

coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID),

Name: randomUsername(),

mafredri added a commit that referenced this issue Jul 20, 2022
We are starting to run into test flakes due to lack of randomness in CI,
this change simply bumps randomness by additional suffix numbers.

See: #3038 (comment)
mafredri added a commit that referenced this issue Jul 20, 2022
We are starting to run into test flakes due to lack of randomness in CI,
this change simply bumps randomness by additional suffix numbers.

See: #3038 (comment)
@spikecurtis
Copy link
Contributor

c.f. #1561

@mafredri
Copy link
Member Author

Thanks @spikecurtis, as described #1561 would solve the reported issue. There is one other place in particular where we would run into this issue:

Name: namesgenerator.GetRandomName(1),

ALTER TABLE ONLY template_versions
    ADD CONSTRAINT template_versions_template_id_name_key UNIQUE (template_id, name);

Then there's also this one, which might be lower priority since it's tied to the in-memory database?

name := namesgenerator.GetRandomName(1)

ALTER TABLE ONLY provisioner_daemons
    ADD CONSTRAINT provisioner_daemons_name_key UNIQUE (name);

There are a few other uses of namesgenerator.GetRandomName which would go away by fixing #1561.

@kylecarbs
Copy link
Member

Closing this in favor of #1561. The context here is helpful!

@kylecarbs kylecarbs closed this as not planned Won't fix, can't repro, duplicate, stale Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Area: HTTP API
Projects
None yet
Development

No branches or pull requests

3 participants