Fixes #4948 | Lazy event parsing #4986

janisz · 2017-01-18T21:26:43Z

Change SSE event stream handler to use events that are
lazy parsed to JSON. This will reduce CPU time when most events
are filtered and/or when more then one subscriper is connected.

mesosphere-ci · 2017-01-18T21:26:44Z

Can one of the admins verify this patch?

mesosphere-ci · 2017-01-18T21:26:45Z

Can one of the admins verify this patch?

jasongilanfarr

Looks pretty good, I think it could actually be a fair amount simpler

jasongilanfarr · 2017-01-20T00:47:04Z

src/main/scala/mesosphere/marathon/core/event/Events.scala

+  val eventType: String = event.eventType
+  lazy val message: String = Json.stringify(eventToJson(event))
+}
+
 sealed trait MarathonEvent {
  val eventType: String
  val timestamp: String


Could have all of these have lazy val jsonString....

I'm sorry I don't understand. If you are referring to put lazy val jsonString into MarathonEvent Then we need to ignore this filed during serialization. I created new class to distinct data from their view. We use JSON now but we can use Protobuf or something else in the future. Should I move JSONEvent to SSE Actor?

sorry for forgetting about this... since we control the serialization/deserialization of these, we could have the jsonString as a member and not serialize the field.

jasongilanfarr · 2017-01-20T00:47:25Z

src/main/scala/mesosphere/marathon/core/event/impl/stream/HttpEventStreamActor.scala

@@ -16,7 +17,7 @@ import scala.util.Try
 trait HttpEventStreamHandle {
  def id: String
  def remoteAddress: String
-  def sendEvent(event: String, message: String): Unit
+  def sendEvent(event: Event): Unit


And then force these to be MarathonEvent

jasongilanfarr · 2017-01-20T00:47:43Z

src/main/scala/mesosphere/marathon/core/event/impl/stream/HttpEventStreamHandleActor.scala

@@ -57,10 +55,10 @@ class HttpEventStreamHandleActor(
  private[this] def sendAllMessages(): Unit = {
    if (outstanding.nonEmpty) {
      val toSend = outstanding.reverse
-      outstanding = List.empty[MarathonEvent]
+      outstanding = List.empty[JsonEvent]


Prefer Seq.empty over List.empty

jasongilanfarr · 2017-01-20T00:48:23Z

src/main/scala/mesosphere/marathon/core/event/impl/stream/HttpEventStreamHandleActor.scala

@@ -5,11 +5,9 @@ import java.io.EOFException
 import akka.actor.{ Actor, ActorLogging, Status }


Can you change the package into a double declaration?

e.g.

package mesosphere.marathon package core.event.impl.stream```

jasongilanfarr · 2017-01-20T00:49:13Z

src/main/scala/mesosphere/marathon/core/event/Events.scala

@@ -1,6 +1,7 @@
 package mesosphere.marathon.core.event


Can you double package this too?

jasongilanfarr · 2017-01-20T00:49:37Z

ok to test

janisz · 2017-01-30T14:10:26Z

Rebased

Change SSE event stream handler to use events that are lazy parsed to JSON. This will reduce CPU time when most events are filtered and/or when more then one subscriper is connected.

jasongilanfarr

Thanks!

unterstein

LGTM
Thanks!

meichstedt

Thanks!

unterstein · 2017-02-06T13:10:46Z

Thanks @janisz !

    .  o ..                  
     o . o o.o                
          ...oo               
            __[]__          Shippin'  
         __|_o_o_o\__         it!
         \""""""""""/         
          \. ..  . /          
     ^^^^^^^^^^^^^^^^^^^^

Change SSE event stream handler to use events that are lazy parsed to JSON. This will reduce CPU time when most events are filtered and/or when more then one subscriper is connected.

Cherry-picked: de22344

Change SSE event stream handler to use events that are lazy parsed to JSON. This will reduce CPU time when most events are filtered and/or when more then one subscriper is connected.

janisz force-pushed the fixes/#4948/lazy_event_parsing branch 2 times, most recently from 6ae7526 to a568f92 Compare January 18, 2017 22:12

jasongilanfarr suggested changes Jan 20, 2017

View reviewed changes

janisz force-pushed the fixes/#4948/lazy_event_parsing branch from 27065b0 to a4cb973 Compare January 23, 2017 12:11

janisz force-pushed the fixes/#4948/lazy_event_parsing branch from a4cb973 to 33a52c1 Compare January 30, 2017 14:10

janisz mentioned this pull request Jan 30, 2017

Create Marathon 1.3.9 #5035

Closed

Fixes d2iq-archive#4948 | Lazy event parsing

de22344

Change SSE event stream handler to use events that are lazy parsed to JSON. This will reduce CPU time when most events are filtered and/or when more then one subscriper is connected.

janisz force-pushed the fixes/#4948/lazy_event_parsing branch from 33a52c1 to de22344 Compare January 30, 2017 21:48

unterstein mentioned this pull request Jan 30, 2017

Marathon 1.3.10 #5039

Closed

jasongilanfarr approved these changes Jan 31, 2017

View reviewed changes

Merge branch 'master' into fixes/d2iq-archive#4948/lazy_event_parsing

e550cc3

unterstein mentioned this pull request Feb 6, 2017

Release Marathon v1.3.10 #5109

Closed

Merge branch 'master' into fixes/d2iq-archive#4948/lazy_event_parsing

c1e3933

unterstein approved these changes Feb 6, 2017

View reviewed changes

meichstedt approved these changes Feb 6, 2017

View reviewed changes

unterstein merged commit cbb1df7 into d2iq-archive:master Feb 6, 2017

janisz deleted the fixes/#4948/lazy_event_parsing branch February 6, 2017 13:45

janisz added a commit to janisz/marathon that referenced this pull request Feb 6, 2017

Fixes d2iq-archive#4948 | Lazy event parsing d2iq-archive#4986

ec2fdb8

Cherry-picked: de22344

unterstein mentioned this pull request Mar 3, 2017

List bugfixes on master not on releases/1.4 #5292

Closed

marcomonaco added the pr label Mar 6, 2017

janisz mentioned this pull request Jan 24, 2018

Serialize events only once. #5927

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #4948 | Lazy event parsing #4986

Fixes #4948 | Lazy event parsing #4986

janisz commented Jan 18, 2017

mesosphere-ci commented Jan 18, 2017

mesosphere-ci commented Jan 18, 2017

jasongilanfarr left a comment

jasongilanfarr Jan 20, 2017

janisz Jan 20, 2017

jasongilanfarr Jan 30, 2017

janisz Jan 30, 2017

jasongilanfarr Jan 20, 2017

jasongilanfarr Jan 20, 2017

janisz Jan 23, 2017

jasongilanfarr Jan 20, 2017

janisz Jan 23, 2017

jasongilanfarr Jan 20, 2017

jasongilanfarr commented Jan 20, 2017

janisz commented Jan 30, 2017

jasongilanfarr left a comment

unterstein left a comment

meichstedt left a comment

unterstein commented Feb 6, 2017

		@@ -5,11 +5,9 @@ import java.io.EOFException
		import akka.actor.{ Actor, ActorLogging, Status }

Fixes #4948 | Lazy event parsing #4986

Fixes #4948 | Lazy event parsing #4986

Conversation

janisz commented Jan 18, 2017

mesosphere-ci commented Jan 18, 2017

mesosphere-ci commented Jan 18, 2017

jasongilanfarr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasongilanfarr commented Jan 20, 2017

janisz commented Jan 30, 2017

jasongilanfarr left a comment

Choose a reason for hiding this comment

unterstein left a comment

Choose a reason for hiding this comment

meichstedt left a comment

Choose a reason for hiding this comment

unterstein commented Feb 6, 2017