Skip to content

remotecache: make sure local writer commits close grpc stream#2025

Closed
tonistiigi wants to merge 1 commit intomoby:masterfrom
tonistiigi:local-cache-stream-close
Closed

remotecache: make sure local writer commits close grpc stream#2025
tonistiigi wants to merge 1 commit intomoby:masterfrom
tonistiigi:local-cache-stream-close

Conversation

@tonistiigi
Copy link
Copy Markdown
Member

@AkihiroSuda I randomly get

ERRO[0004] (*service).Write failed                       error="rpc error: code = Canceled desc = context canceled" expected="sha256:418cf4f2671cfbe7391c3bc89ba9e9b34c3209a742ab0527c08bf6efc4d83bc7" ref="sha256:418cf4f2671cfbe7391c3bc89ba9e9b34c3209a742ab0527c08bf6efc4d83bc7" total=563

on client side while exporting to the local cache. iiuc this is because the exporter doesn't cleanly close the grpc writer and so the loop that reads from it in the client side might close with cancellation error after build has completed and session.Close has been called.

As this is in vendored code I'm not sure how it is supposed to work or if this change is really missing in upstream.

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
rw.offset = resp.Offset
return nil

return rw.Close()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be also called in defer

Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Mar 16, 2021

Choose a reason for hiding this comment

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

(On second thought, defer close might not be needed)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think commit should be "finalized" even if an error is returned

@tonistiigi
Copy link
Copy Markdown
Member Author

Upstream fix has been vendored in

@tonistiigi tonistiigi closed this Apr 26, 2021
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