This repository was archived by the owner on May 25, 2021. It is now read-only.
Address Connection: Close In Headers #42
Open
Conversation
This file contains hidden or 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
added 2 commits
June 27, 2016 18:43
An older version of ibrowse would throw a {error, {'EXIT', Reason}},
when a connection:closed header was received. In the newer version
of ibrowse, it throws {error, req_timedout} instead. This leads
to a maybe_retry function call because we do not have a clause
that handles this error. Which inevitably leads to the replication
process dying once it exhausts the retry limit. So we change
the process_response clause to address this bug. However, this also
means we could end up trying forever for real timeouts.
BugzId:69053
When a connection:closed header is sent from the server, we handle it by calling ibrowse:stop on the worker and release it from the worker pool. But our clean_mailbox tries to clean the mailbox of this worker when it's already dead, leading to a timeout that crashes the changes_reader process and subsequently the replicator process. So we check to ensure that the Worker is still alive before we call ibrowse:stream_next. BugzId:69053
Member
|
it's |
tonysun83
changed the title
Member
|
Agree. Update that in both commit messages though and I'd be +1 |
Member
|
I fixed up the comments before merging. |
asfgit
pushed a commit
that referenced
this pull request
Mar 14, 2017
Using messages instead of callbacks avoids having to spawn another gen_server and then proxying calls through it. To handle death of event source, monitor event server and stop main process if that server dies. For unit test, meck doesn't know how to mock the monitor function, so have to create an actual process for it to monitor.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
This was encountered when testing replication with a proxy server that injected Connection:close headers in a response. Two issues arise in our replicator when we see this in the header:
An older version of ibrowse would throw a {error, {'EXIT', normal}}, when a connection:closed header was received. In the newer version of ibrowse, it throws {error, req_timedout} instead. This leads to a maybe_retry function call because we do not have a clause that handles this error. Which inevitably leads to the replication process dying once it exhausts the retry limit. So we change
the process_response clause to address this bug. However, this also means we could end up trying forever for real timeouts. If there is a way to distinguish between a real timeout vs a closed:connection, I'm open for sugesstions.
We call ibrowse:stop on the worker and release it from the worker pool. But our clean_mailbox tries to clean the mailbox of this worker when it's already dead, leading to a timeout that crashes the replicator. So we check to ensure that the Worker is still alive before we call ibrowse:stream_next.