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

Cancellation2 #36

Closed
wants to merge 11 commits into from
Closed

Cancellation2 #36

wants to merge 11 commits into from

Conversation

vu3rdd
Copy link

@vu3rdd vu3rdd commented Nov 16, 2021

Code Review Checklist (to be filled out by reviewer)

  • Description accurately reflects what changes are being made.
  • Description explains why the changes are being made (or references an issue containing one).
  • The PR appropriately sized.
  • New code has enough tests.
  • New code has enough documentation to answer "how do I use it?" and "what does it do?".
  • Existing documentation is up-to-date, if impacted.

psanford and others added 11 commits November 1, 2021 13:06
@psanford @vu3rdd
Handle context cancels of pending connections
e75755f 
Previously if one side attempted to connect and the other side never
joined, the client would block waiting for rendezvous messages and
canceling would do nothing.

Now if the client is waiting for a message from the rendezvous server,
a context cancel will unblock the client and trigger an error.

Closes: #50 [via git-merge-pr]
@vu3rdd
get the tests to compile
bd046a3
@vu3rdd
go fmt
13c65b7
@vu3rdd
do not create new contexts (and some other experiments..)
de93072
@vu3rdd
send side cancellation is working
eb79532
@vu3rdd
do we need to close channel and connection here?
6a87c94
@vu3rdd
close connection on cancel and also no need to create another cancel …
87600b3 
…context
@vu3rdd
close connection when context is cancelled
33af1d1
@vu3rdd
remove extraneous prints
0c4912e
@vu3rdd
goroutine to monitor cancellation is needed to propagate the error up
cf826ee
@vu3rdd
debug print for CI i286 test failure
b074c30
@vu3rdd vu3rdd closed this Jan 25, 2022
@vu3rdd
Copy link
Author

vu3rdd commented Jan 25, 2022

Already addressed by #44

@vu3rdd vu3rdd deleted the cancellation2 branch March 23, 2022 12:01
JustusFT pushed a commit that referenced this pull request Aug 18, 2022
@psanford
Attempt to deflake rendezvous tests
f7b51a1 
We see occasional failures on Github CI for this test where we haven't
gotten any agent strings back. This appears to be a race condition
between the server ack'ing the message and then saving the user agent
string. Move the ack after the save to ensure its there in the test.

Closes: #36 [via git-merge-pr]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vu3rdd @psanford