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

Work around concurrent map write issue in tests. #2611

Merged
merged 1 commit into from
Feb 9, 2022

Conversation

sipsma
Copy link
Collaborator

@sipsma sipsma commented Feb 9, 2022

The merge diff tests were using some common LLB state objects across
multiple test cases, which each have their own buildkit client and ran
in parallel. The LLB client library does not appear to currently work
with such use cases where LLB states are shared, with errors about
concurrent map writes to marshal state constraints occasionally being
hit during test runs.

Until that issue is addressed (or it's made explicit that the LLB client
library is not expected to work in such a way), this works around the
problem by not sharing states and instead using common funcs that return
distinct state instances.

Signed-off-by: Erik Sipsma erik@sipsma.dev

The issue this is working around is tracked here.

The merge diff tests were using some common LLB state objects across
multiple test cases, which each have their own buildkit client and ran
in parallel. The LLB client library does not appear to currently work
with such use cases where LLB states are shared, with errors about
concurrent map writes to marshal state constraints occasionally being
hit during test runs.

Until that issue is addressed (or it's made explicit that the LLB client
library is not expected to work in such a way), this works around the
problem by not sharing states and instead using common funcs that return
distinct state instances.

Signed-off-by: Erik Sipsma <erik@sipsma.dev>
@sipsma sipsma merged commit 2f99651 into moby:master Feb 9, 2022
@sipsma sipsma deleted the source-map-panic branch February 9, 2022 01:59
@crazy-max
Copy link
Member

@sipsma Are you aware of this one https://github.com/moby/buildkit/runs/5194094096?check_suite_focus=true#step:6:1058?:

=== CONT  TestIntegration/TestDiffOfMerges/worker=containerd-snapshotter-stargz
    mergediff_test.go:1285: 
        	Error Trace:	client_test.go:4696
        	            				mergediff_test.go:1285
        	            				run.go:201
        	Error:      	Received unexpected error:
        	            	rpc error: code = Unavailable desc = error reading from server: EOF
        	            	failed to receive status
        	            	github.com/moby/buildkit/client.(*Client).solve.func4
        	            		/src/client/solve.go:261
        	            	golang.org/x/sync/errgroup.(*Group).Go.func1
        	            		/src/vendor/golang.org/x/sync/errgroup/errgroup.go:57
        	            	runtime.goexit
        	            		/usr/local/go/src/runtime/asm_amd64.s:1581
        	Test:       	TestIntegration/TestDiffOfMerges/worker=containerd-snapshotter-starg

@sipsma
Copy link
Collaborator Author

sipsma commented Feb 15, 2022

@crazy-max No that looks to be an unrelated issue with the progress controller, thanks for letting me know. PR for a fix to it is here: #2641

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