Timed out operations are removed from funnel #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sync upstream repo
ollana
reviewed
Mar 10, 2020
funnel.go
Outdated
| @@ -38,6 +42,12 @@ type operationInProcess struct { | |||
|
|
|||
| // The result from executing of operation, will be available after the channel will be closed. | |||
| opResult | |||
|
|
|||
| // true when this operation has been deleted from the funnel | |||
| deleted abool.AtomicBool | |||
There was a problem hiding this comment.
according to "abool" documentation, we should use pointer to avoid copy
funnel.go
Outdated
| op, found := f.opInProcess[operationId] | ||
| if !found { | ||
| panic("unexpected behavior, operation id not found") | ||
| //Check if the operation completed after a timeout which would result in the operation being deleted from the funnelhas already deleted the the operation from the funnel. |
| wg.Wait() | ||
|
|
||
| elapsedTimeAllRequests :=time.Since(start) | ||
| expectedOperationTimeoutWithGrace:= funnelTimeout + time.Duration(100*time.Millisecond) |
There was a problem hiding this comment.
expectedOperationTimeoutWithGrace would be 600 milliseconds, when operationSleepTime is 550. The test doesn't insure the operation ended with a timeout.
Im suggesting to validate that timeout error returned for each request.
There was a problem hiding this comment.
I'm assuming funnel timeout is working. Perhaps we need additional tests to assure this.
ollana
approved these changes
Mar 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.