Comments (6)
I will provide feedback. The code has been running all weekend, so I need to review some logs to make sure everything is working as expected. Then I'll report my findings.
from go-marathon.
Hopefully the leak can be solved with the same pattern described in #198 (comment), which is to introduce a done
channel that the receiver may use as a termination indicator.
from go-marathon.
I found a bug in the streams library you use that also causes leaks and reported it here. I patched the local copy I have an am testing it now, but I mention it because I wrapped the proposed close method in some code that should stop the goroutine. Also, I wanted you to be aware of this so that you could incorporate that fix into your code as well. Below is the snippet I created to fix the bug:
if len(r.listeners) == 0 {
r.debugLog.Printf("registerSSESubscription(): stopping due to no listeners found.\n")
r.subscribedToSSE = false
stream.Close() // Call proposed in issue #6 in donovanhide/streams.
break
}
From what I see in your code, this should work as it will shut down the goroutine when there are no listeners, but when one is added, it will be restarted. I'm testing this now and it looks like this is working, however I accidentally forgot a line of code so it still crashed after a random amount of time, so I can't say for certain. I'll let you know the results after I let this run a good long time.
from go-marathon.
@sybrandy thanks for the pointer to the upstream issue. I noticed that the eventsource maintainer left a few comments on the original PR that's presumably being revived now. Now that you are testing the patch, I wonder if you also intend to address those comments so that we can get a proper fix in?
Thanks!
from go-marathon.
Hi @timoreimann @sybrandy @gambol99 . I was going to ask similar questions until I found this thread.
There are two scenarios when the connection between go-marathon and Marathon Event Bus should be closed:
- Applications want to close the connection for whatever reason (Currently there is no solution for the scenario as this PR stated. )
- There are errors between the client and server, the connection should be closed and try with another Marathon node.
I purposed a PR for scenario 2 to the underlying eventsource library(basically by adding a timeout and then close the channels when timeout is reached) and the maintainer of that lib suggests to do the timeout at go-marathon side and send the signal to eventsource to close the channels. I am a little bit sceptical about the suggestion and wonder you guys' advice.
More discussions can be seen here: donovanhide/eventsource#24
Thanks for the reply!
from go-marathon.
@iandyh thanks for the pointer to the eventsource PR and discussion. I have left a comment there.
from go-marathon.
Related Issues (20)
- TestRegisterSEESubscriptionReconnectsStreamOnError fails on windows HOT 7
- Example panics on Marathon 1.4
- Application API should support `embed` arguments HOT 2
- Missing Version API HOT 1
- latest master fails to compile HOT 3
- Marathon hosts marked as down never become healthy with Marathon 1.4.4 HOT 3
- Http client configuration no longer honored for event stream connections HOT 5
- When a member is marked down, we ping Marathon to death HOT 3
- Marathon 1.5 API Support HOT 26
- lastTaskFailure is always nil HOT 1
- example in readme doesn't compile HOT 2
- Issues regarding to SSE events HOT 5
- Remove dependency on plan field
- Client should be updated to follow /v2/events redirection responses
- Can wipe task support?
- Closing SSE Subscriptions
- Support for fetching files from task sandbox HOT 3
- MESOS Container Type Support
- Support CSI Volume definition
- Consider making a new release?
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from go-marathon.