Add service canary #392
Add service canary #392
Conversation
|
Updated as comment @samutamm |
| logger.Warnf("dead instances: %s", serviceInstances(deadInstances)) | ||
| } | ||
| if len(rebornInstances) > 0 { | ||
| logger.Warnf("reborn instances: %s", serviceInstances(rebornInstances)) |
There was a problem hiding this comment.
should this log message be a info ?
| // GetServiceCanaryWithInfo gets the service canary with raw info. | ||
| func (s *Service) GetServiceCanaryWithInfo(serviceCanaryName string) (*spec.ServiceCanary, *mvccpb.KeyValue) { |
There was a problem hiding this comment.
this function is only called in GetServiceCanary and WithInfo is not good name, I propose to remove this function and only keep GetServiceCanary
| // Priority must be [1, 9], the default is 5 if user does not set it. | ||
| // The smaller number get higher priority. | ||
| // The order is sorted by name alphabetically in the same priority. |
There was a problem hiding this comment.
why limit priority to [1, 9], is the range large enough?
There was a problem hiding this comment.
Enough for our scenarios. We leverage the OS scheduling system plus explicit two-layer sorting. Infinite priority with integer causes a bad user experience.
| return nil | ||
| } | ||
|
|
||
| // Clone clonse TrafficRules. |
There was a problem hiding this comment.
| // Clone clonse TrafficRules. | |
| // Clone clones TrafficRules. |
| break | ||
| } | ||
|
|
||
| headerMatchNum := 0 |
There was a problem hiding this comment.
the change in this function impact performance.
Now, when MatchAllHeaders is false, the code will continue to match other headers when one header is matched.
There was a problem hiding this comment.
Updated with a pre-checking.
Codecov Report
@@ Coverage Diff @@
## main #392 +/- ##
==========================================
- Coverage 80.82% 80.37% -0.45%
==========================================
Files 60 62 +2
Lines 6966 7107 +141
==========================================
+ Hits 5630 5712 +82
- Misses 1045 1100 +55
- Partials 291 295 +4
Continue to review full report at Codecov.
|
290a064
to
c27dbb4
Compare
|
I resolved all conflicts, please review @suchen-sci @samutamm @zhao-kun |
| select { | ||
| case egs.chReloadEvent <- struct{}{}: | ||
| } |
There was a problem hiding this comment.
| select { | |
| case egs.chReloadEvent <- struct{}{}: | |
| } | |
| egs.chReloadEvent <- struct{}{} |
There was a problem hiding this comment.
No need to change, We keep the pattern consistent
There was a problem hiding this comment.
Got it. My static checker just report this. FYI, https://staticcheck.io/docs/checks#S1000
message: "should use a simple channel send/receive instead of select with a single case (S1000) go-staticcheck".
There was a problem hiding this comment.
Updated with correct select statement
(Will make actions correct after megaease/easemesh-api#16 merged)
&to more idiomatic,.