pbacsko commented on code in PR #579:
URL: https://github.com/apache/yunikorn-core/pull/579#discussion_r1257323642
##########
pkg/events/event_system.go:
##########
@@ -61,13 +102,115 @@ func CreateAndSetEventSystem() {
stopped: false,
publisher: CreateShimPublisher(store),
}
+
+ if eventSystemImpl, ok := ev.(*EventSystemImpl); ok {
+ eventSystemImpl.eventSystemId = fmt.Sprintf("event-system-%p",
eventSystemImpl)
+ ev = eventSystemImpl
+ }
}
func (ec *EventSystemImpl) StartService() {
ec.StartServiceWithPublisher(true)
}
+func getConfigurationBool(key string, defaultValue bool) bool {
+ value, ok := configs.GetConfigMap()[key]
+ if !ok {
+ return defaultValue
+ }
+ boolValue, err := strconv.ParseBool(value)
+ if err != nil {
+ log.Log(log.Events).Warn("Failed to parse configuration value",
+ zap.String("key", key),
+ zap.String("value", value),
+ zap.Error(err))
+ return defaultValue
+ }
+ return boolValue
+}
+
+func getConfigurationInt(key string, defaultValue int) int {
+ value, ok := configs.GetConfigMap()[key]
+ if !ok {
+ return defaultValue
+ }
+ intVal, err := strconv.ParseInt(value, 10, 32)
+ if err != nil {
+ log.Log(log.Events).Warn("Failed to parse configuration value",
+ zap.String("key", key),
+ zap.String("value", value),
+ zap.Error(err))
+ return defaultValue
+ }
+ return int(intVal)
+}
+
+func (ec *EventSystemImpl) readIsRequestsEnabled() bool {
+ return getConfigurationBool(configs.CMEventRequestsEnabled,
configs.DefaultEventRequestsEnabled)
+}
+
+func (ec *EventSystemImpl) readIsTrackingEnabled() bool {
+ return getConfigurationBool(configs.CMEventTrackingEnabled,
configs.DefaultEventTrackingEnabled)
+}
+
+func (ec *EventSystemImpl) readRequestCapacity() int {
+ return getConfigurationInt(configs.CMEventRequestCapacity,
configs.DefaultEventRequestCapacity)
+}
+
+func (ec *EventSystemImpl) readBufferCapacity() int {
+ return getConfigurationInt(configs.CMEventBufferCapacity,
configs.DefaultEventBufferCapacity)
+}
+
+func (ec *EventSystemImpl) readResponseMaxSize() int {
+ return getConfigurationInt(configs.CMEventResponseSize,
configs.DefaultEventResponseSize)
+}
+
+func (ec *EventSystemImpl) isRestartNeeded() bool {
+ ec.Lock()
+ defer ec.Unlock()
+
+ requestsEnabled := ec.readIsRequestsEnabled()
+ trackingEnabled := ec.readIsTrackingEnabled()
+ requestCapacity := ec.readRequestCapacity()
+ bufferCapacity := ec.readBufferCapacity()
+ responseMaxSize := ec.readResponseMaxSize()
+
+ if requestsEnabled != ec.requestsEnabled ||
+ trackingEnabled != ec.trackingEnabled ||
+ requestCapacity != ec.requestCapacity ||
+ bufferCapacity != ec.bufferCapacity ||
+ responseMaxSize != ec.responseMaxSize {
+ return true
+ }
+
+ return false
+}
+
+func (ec *EventSystemImpl) Restart() {
+ ec.Stop()
+ ec.StartServiceWithPublisher(true)
+}
+
+func (ec *EventSystemImpl) reloadConfig() {
+ if ec.isRestartNeeded() {
+ ec.Restart()
+ }
+}
Review Comment:
I think our position in the design was to not make this reconfigurable. This
affects existing entries in the ringbuffer, so if we want to support it, at
least we need to deal with the size change. So existing events need to be
copied and kept, that's the bare minimum. It's also much easier to reason about
the enabled/disabled status if it cannot change, eg. we just have a simple
`bool` and don't need an atomic boolean or anything which can change (see #581,
#574, etc).
cc @wilfred-s
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
