Optimize etcd storage by compressing recurring events in to a single event

2026-01-04 23:17:50 +00:00 · 2015-02-10 16:49:32 -08:00
parent 52bf48cac2
commit 033577efa2
8 changed files with 535 additions and 38 deletions
--- a/pkg/client/record/event_test.go
+++ b/pkg/client/record/event_test.go
@@ -36,19 +36,24 @@ func init() {
 }

 type testEventRecorder struct {
-	OnEvent func(e *api.Event) (*api.Event, error)
+	OnCreate func(e *api.Event) (*api.Event, error)
+	OnUpdate func(e *api.Event) (*api.Event, error)
 }

 // CreateEvent records the event for testing.
 func (t *testEventRecorder) Create(e *api.Event) (*api.Event, error) {
-	if t.OnEvent != nil {
-		return t.OnEvent(e)
+	if t.OnCreate != nil {
+		return t.OnCreate(e)
 	}
 	return e, nil
 }

-func (t *testEventRecorder) clearOnEvent() {
-	t.OnEvent = nil
+// UpdateEvent records the event for testing.
+func (t *testEventRecorder) Update(e *api.Event) (*api.Event, error) {
+	if t.OnUpdate != nil {
+		return t.OnUpdate(e)
+	}
+	return e, nil
 }

 func TestEventf(t *testing.T) {
@@ -60,17 +65,28 @@ func TestEventf(t *testing.T) {
 			UID:       "bar",
 		},
 	}
+	testPod2 := &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			SelfLink:  "/api/v1beta1/pods/foo",
+			Name:      "foo",
+			Namespace: "baz",
+			UID:       "differentUid",
+		},
+	}
+
 	testRef, err := api.GetPartialReference(testPod, "desiredState.manifest.containers[2]")
+	testRef2, err := api.GetPartialReference(testPod2, "desiredState.manifest.containers[3]")
 	if err != nil {
 		t.Fatal(err)
 	}
 	table := []struct {
-		obj        runtime.Object
-		reason     string
-		messageFmt string
-		elements   []interface{}
-		expect     *api.Event
-		expectLog  string
+		obj          runtime.Object
+		reason       string
+		messageFmt   string
+		elements     []interface{}
+		expect       *api.Event
+		expectLog    string
+		expectUpdate bool
 	}{
 		{
 			obj:        testRef,
@@ -95,10 +111,36 @@ func TestEventf(t *testing.T) {
 				Source:  api.EventSource{Component: "eventTest"},
 				Count:   1,
 			},
-			expectLog: `Event(api.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1beta1", ResourceVersion:"", FieldPath:"desiredState.manifest.containers[2]"}): reason: 'Started' some verbose message: 1`,
+			expectLog:    `Event(api.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1beta1", ResourceVersion:"", FieldPath:"desiredState.manifest.containers[2]"}): reason: 'Started' some verbose message: 1`,
+			expectUpdate: false,
 		},
 		{
 			obj:        testPod,
+			reason:     "Killed",
+			messageFmt: "some other verbose message: %v",
+			elements:   []interface{}{1},
+			expect: &api.Event{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "foo",
+					Namespace: "baz",
+				},
+				InvolvedObject: api.ObjectReference{
+					Kind:       "Pod",
+					Name:       "foo",
+					Namespace:  "baz",
+					UID:        "bar",
+					APIVersion: "v1beta1",
+				},
+				Reason:  "Killed",
+				Message: "some other verbose message: 1",
+				Source:  api.EventSource{Component: "eventTest"},
+				Count:   1,
+			},
+			expectLog:    `Event(api.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1beta1", ResourceVersion:"", FieldPath:""}): reason: 'Killed' some other verbose message: 1`,
+			expectUpdate: false,
+		},
+		{
+			obj:        testRef,
 			reason:     "Started",
 			messageFmt: "some verbose message: %v",
 			elements:   []interface{}{1},
@@ -113,37 +155,140 @@ func TestEventf(t *testing.T) {
 					Namespace:  "baz",
 					UID:        "bar",
 					APIVersion: "v1beta1",
+					FieldPath:  "desiredState.manifest.containers[2]",
+				},
+				Reason:  "Started",
+				Message: "some verbose message: 1",
+				Source:  api.EventSource{Component: "eventTest"},
+				Count:   2,
+			},
+			expectLog:    `Event(api.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1beta1", ResourceVersion:"", FieldPath:"desiredState.manifest.containers[2]"}): reason: 'Started' some verbose message: 1`,
+			expectUpdate: true,
+		},
+		{
+			obj:        testRef2,
+			reason:     "Started",
+			messageFmt: "some verbose message: %v",
+			elements:   []interface{}{1},
+			expect: &api.Event{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "foo",
+					Namespace: "baz",
+				},
+				InvolvedObject: api.ObjectReference{
+					Kind:       "Pod",
+					Name:       "foo",
+					Namespace:  "baz",
+					UID:        "differentUid",
+					APIVersion: "v1beta1",
+					FieldPath:  "desiredState.manifest.containers[3]",
 				},
 				Reason:  "Started",
 				Message: "some verbose message: 1",
 				Source:  api.EventSource{Component: "eventTest"},
 				Count:   1,
 			},
-			expectLog: `Event(api.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1beta1", ResourceVersion:"", FieldPath:""}): reason: 'Started' some verbose message: 1`,
+			expectLog:    `Event(api.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"differentUid", APIVersion:"v1beta1", ResourceVersion:"", FieldPath:"desiredState.manifest.containers[3]"}): reason: 'Started' some verbose message: 1`,
+			expectUpdate: false,
+		},
+		{
+			obj:        testRef,
+			reason:     "Started",
+			messageFmt: "some verbose message: %v",
+			elements:   []interface{}{1},
+			expect: &api.Event{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "foo",
+					Namespace: "baz",
+				},
+				InvolvedObject: api.ObjectReference{
+					Kind:       "Pod",
+					Name:       "foo",
+					Namespace:  "baz",
+					UID:        "bar",
+					APIVersion: "v1beta1",
+					FieldPath:  "desiredState.manifest.containers[2]",
+				},
+				Reason:  "Started",
+				Message: "some verbose message: 1",
+				Source:  api.EventSource{Component: "eventTest"},
+				Count:   3,
+			},
+			expectLog:    `Event(api.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1beta1", ResourceVersion:"", FieldPath:"desiredState.manifest.containers[2]"}): reason: 'Started' some verbose message: 1`,
+			expectUpdate: true,
+		},
+		{
+			obj:        testRef2,
+			reason:     "Stopped",
+			messageFmt: "some verbose message: %v",
+			elements:   []interface{}{1},
+			expect: &api.Event{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "foo",
+					Namespace: "baz",
+				},
+				InvolvedObject: api.ObjectReference{
+					Kind:       "Pod",
+					Name:       "foo",
+					Namespace:  "baz",
+					UID:        "differentUid",
+					APIVersion: "v1beta1",
+					FieldPath:  "desiredState.manifest.containers[3]",
+				},
+				Reason:  "Stopped",
+				Message: "some verbose message: 1",
+				Source:  api.EventSource{Component: "eventTest"},
+				Count:   1,
+			},
+			expectLog:    `Event(api.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"differentUid", APIVersion:"v1beta1", ResourceVersion:"", FieldPath:"desiredState.manifest.containers[3]"}): reason: 'Stopped' some verbose message: 1`,
+			expectUpdate: false,
+		},
+		{
+			obj:        testRef2,
+			reason:     "Stopped",
+			messageFmt: "some verbose message: %v",
+			elements:   []interface{}{1},
+			expect: &api.Event{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "foo",
+					Namespace: "baz",
+				},
+				InvolvedObject: api.ObjectReference{
+					Kind:       "Pod",
+					Name:       "foo",
+					Namespace:  "baz",
+					UID:        "differentUid",
+					APIVersion: "v1beta1",
+					FieldPath:  "desiredState.manifest.containers[3]",
+				},
+				Reason:  "Stopped",
+				Message: "some verbose message: 1",
+				Source:  api.EventSource{Component: "eventTest"},
+				Count:   2,
+			},
+			expectLog:    `Event(api.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"differentUid", APIVersion:"v1beta1", ResourceVersion:"", FieldPath:"desiredState.manifest.containers[3]"}): reason: 'Stopped' some verbose message: 1`,
+			expectUpdate: true,
 		},
 	}

 	for _, item := range table {
 		called := make(chan struct{})
 		testEvents := testEventRecorder{
-			OnEvent: func(event *api.Event) (*api.Event, error) {
-				a := *event
-				// Just check that the timestamp was set.
-				if a.FirstTimestamp.IsZero() || a.LastTimestamp.IsZero() {
-					t.Errorf("timestamp wasn't set")
-				}
-				a.FirstTimestamp = item.expect.FirstTimestamp
-				a.LastTimestamp = item.expect.LastTimestamp
-				// Check that name has the right prefix.
-				if n, en := a.Name, item.expect.Name; !strings.HasPrefix(n, en) {
-					t.Errorf("Name '%v' does not contain prefix '%v'", n, en)
-				}
-				a.Name = item.expect.Name
-				if e, a := item.expect, &a; !reflect.DeepEqual(e, a) {
-					t.Errorf("diff: %s", util.ObjectDiff(e, a))
+			OnCreate: func(event *api.Event) (*api.Event, error) {
+				returnEvent, _ := validateEvent(event, item.expect, t)
+				if item.expectUpdate {
+					t.Errorf("Expected event update(), got event create()")
 				}
 				called <- struct{}{}
-				return event, nil
+				return returnEvent, nil
+			},
+			OnUpdate: func(event *api.Event) (*api.Event, error) {
+				returnEvent, _ := validateEvent(event, item.expect, t)
+				if !item.expectUpdate {
+					t.Errorf("Expected event create(), got event update()")
+				}
+				called <- struct{}{}
+				return returnEvent, nil
 			},
 		}
 		recorder := StartRecording(&testEvents, api.EventSource{Component: "eventTest"})
@@ -165,6 +310,39 @@ func TestEventf(t *testing.T) {
 	}
 }

+func validateEvent(actualEvent *api.Event, expectedEvent *api.Event, t *testing.T) (*api.Event, error) {
+	expectCompression := expectedEvent.Count > 1
+	// Just check that the timestamp was set.
+	if actualEvent.FirstTimestamp.IsZero() || actualEvent.LastTimestamp.IsZero() {
+		t.Errorf("timestamp wasn't set: %#v", *actualEvent)
+	}
+	if actualEvent.FirstTimestamp.Equal(actualEvent.LastTimestamp.Time) {
+		if expectCompression {
+			t.Errorf("FirstTimestamp (%q) and LastTimestamp (%q) must be equal to indicate only one occurance of the event, but were different. Actual Event: %#v", actualEvent.FirstTimestamp, actualEvent.LastTimestamp, *actualEvent)
+		}
+	} else {
+		if !expectCompression {
+			t.Errorf("FirstTimestamp (%q) and LastTimestamp (%q) must be different to indicate event compression happened, but were the same. Actual Event: %#v", actualEvent.FirstTimestamp, actualEvent.LastTimestamp, *actualEvent)
+		}
+	}
+	actualFirstTimestamp := actualEvent.FirstTimestamp
+	actualLastTimestamp := actualEvent.LastTimestamp
+	// Temp clear time stamps for comparison because actual values don't matter for comparison
+	actualEvent.FirstTimestamp = expectedEvent.FirstTimestamp
+	actualEvent.LastTimestamp = expectedEvent.LastTimestamp
+	// Check that name has the right prefix.
+	if n, en := actualEvent.Name, expectedEvent.Name; !strings.HasPrefix(n, en) {
+		t.Errorf("Name '%v' does not contain prefix '%v'", n, en)
+	}
+	actualEvent.Name = expectedEvent.Name
+	if e, a := expectedEvent, actualEvent; !reflect.DeepEqual(e, a) {
+		t.Errorf("diff: %s", util.ObjectGoPrintDiff(e, a))
+	}
+	actualEvent.FirstTimestamp = actualFirstTimestamp
+	actualEvent.LastTimestamp = actualLastTimestamp
+	return actualEvent, nil
+}
+
 func TestWriteEventError(t *testing.T) {
 	ref := &api.ObjectReference{
 		Kind:       "Pod",
@@ -210,7 +388,7 @@ func TestWriteEventError(t *testing.T) {

 	defer StartRecording(
 		&testEventRecorder{
-			OnEvent: func(event *api.Event) (*api.Event, error) {
+			OnCreate: func(event *api.Event) (*api.Event, error) {
 				if event.Message == "finished" {
 					close(done)
 					return event, nil
@@ -250,7 +428,7 @@ func TestLotsOfEvents(t *testing.T) {
 	// Fail each event a few times to ensure there's some load on the tested code.
 	var counts [1000]int
 	testEvents := testEventRecorder{
-		OnEvent: func(event *api.Event) (*api.Event, error) {
+		OnCreate: func(event *api.Event) (*api.Event, error) {
 			num, err := strconv.Atoi(event.Message)
 			if err != nil {
 				t.Error(err)