-
Notifications
You must be signed in to change notification settings - Fork 12
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.
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
Frequent connect/disconnect blocks update. #2
Comments
Hi, just to chime in with some information I found concerning this issue (we're in the same boat). If I understand correctly, onebusaway is currently using Jetty 8, which does apparently has some issues with websockets (https://bugs.eclipse.org/bugs/buglist.cgi?quicksearch=websockets). Within some of these issues it's also mentioned that it's fixed in Jetty 9, but not easy to backport to 8. Perhaps it's possible to migrate to this version? (although it isn't the easiest path: http://dev.eclipse.org/mhonarc/lists/jetty-users/msg03689.html and they also changed the supported standards between 8 and 9: http://www.eclipse.org/jetty/documentation/current/websocket-intro.html#ws-intro-provides ) |
Thanks for the updates. Ironically, I had originally implemented this using Jetty 9, but reverted back to 8 so I didn't have to introduce a Java 1.7 requirement. I will see if I can find some time to upgrade to Jetty 9 to see if that helps. |
hopefully fixing problems discussed in Issue #2.
Ok, I've bumped the exporter to use Jetty 9. Note that because this also bumped the library from requiring only Java 1.6 to Java 1.7, I also bumped the library version number to 1.2.0-SNAPSHOT. You'll need to depend on this version to get the latest behavior. Let me know if this helps? |
I can replicate the exact same scenario but now it will resume working after 5 minutes. Afaik the latter wasn't the case in Jetty 8. So it seems we're getting there now just reduce the write timeout? |
I may have spoken too soon, it seems it was only one update, now it's "down" again and i noticed the filewriter was never triggered. I suppose there are still one or more servlets still present in the list with listeners all taking up to the 3 minute time |
|
Another thread dump similar to above but now directly after locking giving less clutter: http://pastebin.com/Zj8xBCuA Most interesting is this:
|
Seems like it's a deadlock in the OBA library instead of just jetty. |
Issue #2: - Made "incremental listeners" array in GtfsRealtimeExporterImpl threadsafe via CopyOnWriteArrayList. This allows us to remove the sychronization around add/removing listeners. The assumption here is that most users of the library will be handling a reasonably small number of clients that don't change that often. If that assumption doesn't hold, we can revisit. - Added some synchronization + cleanup in the actual data socket, in order to more gracefully handle concurrent updates and disconnects. - Added an integration test that attempts to stress the system with a high volume of incremental updates and frequent disconnect/connect by the websocket client.
Ok, I wrote some integration stress-tests and attempted to address the concurrency issues I found as result of that. Maybe give the latest version a try? |
Thanks, to make my life a bit easier could the version be bumped? Don't think the maven artifact got updated, http://nexus.onebusaway.org/content/groups/public/org/onebusaway/onebusaway-gtfs-realtime-exporter/1.2.0-SNAPSHOT/maven-metadata.xml |
Actually, it didn't update because the deadlock test failed on the CI I will have to take a closer look tonight. On Wed, Sep 25, 2013 at 2:18 PM, skywave [email protected] wrote:
|
Ok, this time, the build succeeded on the CI server. Give it a shot. |
The JVM still detects deadlocks but the rest seems to keeps working. It's definitely harder to replicate the old issue, now it's seems only my client starts giving hickups. Did manage to crash the server by putting a simple client in a while loop that kept making new clients, maybe allow to set a max number of clients in the exporter and reject too many clients?
Second attempt
|
Bad stuff still happens when a user leaves the connection without a disconnect. |
Oh and the DeadlockTest fails on my laptop. |
Any logging output you can provide would be super helpful ;) On Fri, Sep 27, 2013 at 3:19 PM, skywave [email protected] wrote:
|
I know :) I was still doing some digging but having troubles with the onebusaway maven repo giving 503's. I actually wanted to do a completely clean run to avoid Maven troubles. Pastebinn'ed as it's too long for github: http://pastebin.com/tgNL7sS9 If i take the _session = [null | session] out of the syncronized block and remove the synchronized block the test passes. Any idea why it was put in a synchronized block? diff --git a/src/main/java/org/onebusaway/gtfs_realtime/exporter/GtfsRealtimeServlet.java b/src/main/java/org/onebusaway/gtfs_realtime/exporter/GtfsRealtimeS
index 7c0d433..e1a0a4a 100644
--- a/src/main/java/org/onebusaway/gtfs_realtime/exporter/GtfsRealtimeServlet.java
+++ b/src/main/java/org/onebusaway/gtfs_realtime/exporter/GtfsRealtimeServlet.java
@@ -131,9 +131,9 @@ public class GtfsRealtimeServlet extends WebSocketServlet implements
@OnWebSocketConnect
public void onOpen(Session session) {
_log.info("client connect");
- synchronized (this) {
+ //synchronized (this) {
_session = session;
- }
+ //}
// When we add ourselves as an incremental listener to the
// GtfsRealtimeSource, it typically triggers a "handleFeed" update
// immediately. Thus, we don't want to call this until we've released the
@@ -146,9 +146,9 @@ public class GtfsRealtimeServlet extends WebSocketServlet implements
public void onClose(Session sesion, int closeCode, String message) {
_log.info("client close");
_source.removeIncrementalListener(this);
- synchronized (this) {
+ //synchronized (this) {
_session = null;
- }
+ //}
} |
If a client fast connect/disconnects, it results in a complete block in new updates. Other clients will no longer receive updates over websockets nor are there new GTFS-RT files written by the FileWriter. The jetty HTTP interface still works but doesn't offer new data.
Possible culprit:
sendmessage method in Jetty hangs forever:
https://github.com/OneBusAway/onebusaway-gtfs-realtime-exporter/blob/master/src/main/java/org/onebusaway/gtfs_realtime/exporter/GtfsRealtimeServlet.java#L138
Resulting in a block of this for loop
https://github.com/OneBusAway/onebusaway-gtfs-realtime-exporter/blob/master/src/main/java/org/onebusaway/gtfs_realtime/exporter/GtfsRealtimeExporterImpl.java#L104
The text was updated successfully, but these errors were encountered: