Make NewMessageCounter async #2464
Conversation
…is prevents value of 0 in metrics
service/counter/message.go
Outdated
| var ( | ||
| messagesCounter prometheus.Counter | ||
| once sync.Once | ||
| ) |
There was a problem hiding this comment.
以下の様にチャネルも定義して、初期化の完了を他のゴルーチンに通知する事で上手く実装出来そうです。
| var ( | |
| messagesCounter prometheus.Counter | |
| once sync.Once | |
| ) | |
| var ( | |
| messagesCounter prometheus.Counter | |
| initOnce sync.Once | |
| initDone = make(chan struct{}) | |
| ) |
service/counter/message.go
Outdated
| go func() { | ||
| for range hub.Subscribe(1, event.MessageCreated).Receiver { |
There was a problem hiding this comment.
初期化チェックを追加
| go func() { | |
| for range hub.Subscribe(1, event.MessageCreated).Receiver { | |
| go func() { | |
| <-initDone | |
| for range hub.Subscribe(1, event.MessageCreated).Receiver { |
以下の様に、Get() の所にも追加するべき。
func (c *messageCounterImpl) Get() int64 {
<-initDone
c.RLock()
defer c.RUnlock()
return c.count
}
service/counter/message.go
Outdated
| }) | ||
| messagesCounter.Add(float64(counter.count)) | ||
| }) |
There was a problem hiding this comment.
初期化が完了したことを通知する
| }) | |
| messagesCounter.Add(float64(counter.count)) | |
| }) | |
| }) | |
| messagesCounter.Add(float64(counter.count)) | |
| close(initDone) | |
| }) |
service/counter/message.go
Outdated
| panic(err) | ||
| } | ||
|
|
||
| once.Do(func() { |
There was a problem hiding this comment.
(少し変数名を変えたので、変更)
| once.Do(func() { | |
| initOnce.Do(func() { |
service/counter/message.go
Outdated
|
|
||
| go func() { | ||
| if err := db.Unscoped().Model(&model.Message{}).Count(&counter.count).Error; err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
panic ではなく、エラーを返すようにしたいな～
initError みたいな変数を用意して格納させるとかで良さそう。
|
機能面は良さそうです！ありがとうございます！ ただ、@motoki317 の https://q.trap.jp/messages/e55d6ed9-ad4e-42bc-bce8-f65844b3f34d を参考にすると、かなりコードを簡素にかつ分かりやすくできると気づいたため、リファクタリングをしました。 具体的には、
|
| initOnce.Do(func() { | ||
| initError = mc.initializeCounter() | ||
| }) | ||
| if initError != nil { | ||
| return nil, initError | ||
| } |
There was a problem hiding this comment.
結局これでは非同期にはなりません...
sync.Onceは、Doを呼んだらそれが返ったときには、ただ一度だけ中の関数の実行が終了しているという性質を持ちます。
↓を参考にしたり、もしよかったらsync.Onceの実装の中身も覗いてみてください。
https://q.trap.jp/messages/50247bdf-5920-4bfa-8a12-c4d789b74792
https://cs.opensource.google/go/go/+/refs/tags/go1.22.5:src/sync/once.go;l=18
(sync.OnceFuncもだいたい同じで、こちらの方が今回の場合は使いやすいかも)
この性質と、promauto.CounterFuncで値をこちら側でコントロールできる性質を活かして、立ち上がり時はブロックしないけど、値を取得するときには初期化が終了していることを表現してみてください。
Fixes #2157