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.

Sign up for GitHub

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

Jump to bottom

distsqlrun: improve outbox error handling #14218

Merged
merged 1 commit into from
Mar 17, 2017
Merged

distsqlrun: improve outbox error handling #14218

merged 1 commit into from
Mar 17, 2017

Conversation

andreimatei
Copy link
Contributor

@andreimatei andreimatei commented Mar 16, 2017 •
edited by rjnn
Loading

Remove the possibility for a consumer to signal an error to a producer.
It wasn't used.

Also improve communication error handling in the outbox and add a test.

This change is Reviewable

@andreimatei andreimatei requested a review from RaduBerinde March 16, 2017 22:57
@vivekmenezes vivekmenezes mentioned this pull request Mar 17, 2017
Merged
@RaduBerinde
Copy link
Member

:lgtm:

Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

pkg/sql/distsqlrun/outbox_test.go, line 338 at r1 (raw file):

				// the server that it's done. We expect the outbox to finish.
				cancel()
				cleanup = func(err error) {

I think we can just defer here instead of using cleanup

Comments from Reviewable

@andreimatei andreimatei force-pushed the distsql-fix-outbox-signal-err branch from 8ff548c to 405fe6d Compare March 17, 2017 17:16
@andreimatei
Copy link
Contributor Author

Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

pkg/sql/distsqlrun/outbox_test.go, line 338 at r1 (raw file):

Previously, RaduBerinde wrote…

I think we can just defer here instead of using cleanup

done. The point of the cleanup was to supply the error that the RunSyncFlow method returns - but that's not tested here.

Comments from Reviewable

@andreimatei
distsqlrun: improve outbox error handling
f6ad9f8 
Remove the possibility for a consumer to signal an error to a producer.
It wasn't used.

Also improve communication error handling in the outbox and add a test.
@andreimatei andreimatei force-pushed the distsql-fix-outbox-signal-err branch from 405fe6d to f6ad9f8 Compare March 17, 2017 18:18
@andreimatei andreimatei merged commit 59e1e6f into cockroachdb:master Mar 17, 2017
@andreimatei andreimatei deleted the distsql-fix-outbox-signal-err branch March 17, 2017 18:37
@rjnn
Copy link
Contributor

rjnn commented Mar 17, 2017

Reviewed 4 of 4 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

Comments from Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaduBerinde RaduBerinde Awaiting requested review from RaduBerinde

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andreimatei @RaduBerinde @rjnn