[Golang] Add restart functionality to go runtime #3447
Conversation
|
@TomerShor I meant these metrics: |
| if currentStatus := g.GetStatus(); currentStatus != status.Ready { | ||
| return nil, errors.Errorf("Runtime not ready (current status: %s)", currentStatus) | ||
| } |
There was a problem hiding this comment.
Adding condition to a critical flow might cause performance hits.
Can you please run benchmark tests to compare the regular flow of with and without the changes?
There was a problem hiding this comment.
Sure, I will do benchmarking.
But anyway,
- we already have this condition in other runtimes
- we will need this condition when we start implementing explicitAck/Termination for go runtime
There was a problem hiding this comment.
@TomerShor results:
1000 total/100 parall:
before: 888.37955ms
after: 841.544612ms
10000 total/100 parall:
before: 955.145597ms
after: 894.964659ms
1000 total/1000 parall:
before: 4.743075748s
after: 4.772192623s
10000 total/1000 parall:
before: 8.847363427s
after: 8.617966213s
There was a problem hiding this comment.
probably, moving of duration metrics calculation could impact performance, so we can see insignificant improve in some cases
Adds restart functionality to go runtime which is crucial to have for kafka runtime to avoid having busy worker after runtime restart. Question to address: Should we add metrics calculation after restart? Because gorotine will still alive on the background for now (We might consider adding some timeout in the future like maxTimeOfEventProcessing), so technically we will have metrics there. jira - https://iguazio.atlassian.net/browse/NUC-338
Adds restart functionality to go runtime which is crucial to have for kafka runtime to avoid having busy worker after runtime restart.
Question to address: Should we add metrics calculation after restart? Because gorotine will still alive on the background for now (We might consider adding some timeout in the future like maxTimeOfEventProcessing), so technically we will have metrics there.
jira - https://iguazio.atlassian.net/browse/NUC-338