Adding stubserver support to tests #11
Conversation
orca/service_test.go
Outdated
| @@ -93,14 +94,32 @@ func (s) TestE2E_CustomBackendMetrics_OutOfBand(t *testing.T) { | |||
| opts := orca.ServiceOptions{MinReportingInterval: shortReportingInterval, ServerMetricsProvider: smr} | |||
| internal.AllowAnyMinReportingInterval.(func(*orca.ServiceOptions))(&opts) | |||
|
|
|||
| // Register the OpenRCAService with a very short metrics reporting interval. | |||
| blockCh := make(chan struct{}) | |||
There was a problem hiding this comment.
why did you introduce a new channel? you can continue to use mutex and define a counter requests as variable
orca/service_test.go
Outdated
| @@ -126,7 +145,7 @@ func (s) TestE2E_CustomBackendMetrics_OutOfBand(t *testing.T) { | |||
| errCh <- fmt.Errorf("UnaryCall failed: %v", err) | |||
| return | |||
| } | |||
| time.Sleep(time.Millisecond) | |||
| time.Sleep(10 * time.Millisecond) | |||
There was a problem hiding this comment.
why increase the sleep time? if you stick the mutex approach, we will make sure all 20 rpc calls are serialized
orca/service_test.go
Outdated
| s := grpc.NewServer() | ||
| if err := orca.Register(s, opts); err != nil { | ||
| t.Fatalf("orca.EnableOutOfBandMetricsReportingForTesting() failed: %v", err) | ||
| t.Fatalf("orca.Register failed: %v", err) |
|
Delete the testserviceimpl code |
…g mutex and count
janardhanvissa
force-pushed
the
stubserversupport
branch
from
364a609 to
4cd651c
Compare
orca/service_test.go
Outdated
| @@ -126,7 +116,10 @@ func (s) TestE2E_CustomBackendMetrics_OutOfBand(t *testing.T) { | |||
| errCh <- fmt.Errorf("UnaryCall failed: %v", err) | |||
| return | |||
| } | |||
| time.Sleep(time.Millisecond) | |||
| mu.Lock() | |||
There was a problem hiding this comment.
why did request counter move here? why can't it remain in stub function?
There was a problem hiding this comment.
By managing the counter outside the stub, we can safely increment it in a concurrent context. This will ensure that we accurately track how many requests have been sent, regardless of their processing order or timing.
There was a problem hiding this comment.
The counter needs to be updated inside of the UnaryCall and t.smr.SetNamedUtilization(requestsMetricKey, float64(t.requests)*0.01) line should remain like earlier. I don't see a point in hard-coding requestsMetricKey to 0.2.
There was a problem hiding this comment.
Done with the above comments. Please re-review once and let me know if any changes are required to be made.
janardhanvissa
force-pushed
the
stubserversupport
branch
from
f2e0298 to
be7bf16
Compare
No description provided.