fix(notifier): fix fetching notifications by batch #974
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #974 +/- ##
==========================================
- Coverage 68.19% 68.12% -0.07%
==========================================
Files 208 208
Lines 11734 11785 +51
==========================================
+ Hits 8002 8029 +27
- Misses 3254 3272 +18
- Partials 478 484 +6 ☔ View full report in Codecov by Sentry. |
database/redis/notification.go
Outdated
| @@ -517,4 +528,22 @@ func (connector *DbConnector) AddNotifications(notifications []*moira.ScheduledN | |||
| return nil | |||
| } | |||
|
|
|||
| func (connector *DbConnector) saveNotifications(ctx context.Context, pipe redis.Pipeliner, notifications []*moira.ScheduledNotification) error { | |||
There was a problem hiding this comment.
есть функция AddNotifications она же такая же...
There was a problem hiding this comment.
Не, AddNotifications добавляет уведомления под одинаковым таймстемпом, а saveNotifications просто сохраняет переданные, ожидая, что все нужные поля заполнены
There was a problem hiding this comment.
я думаю, что мы через пол года уже забудем в чем их отличие, а сходу не будучи в контексте ответить на это, кажется, мы сейчас не умеем :(
давайте завтра обговорим это место :)
There was a problem hiding this comment.
заменил saveNotifications на resaveNotifications, у которого другая логика
|
/build |
|
Build and push Docker images with tag: 2024-02-07.ccb20ab |
1 similar comment
|
Build and push Docker images with tag: 2024-02-07.ccb20ab |
Fix fetching notifications by batch
The mechanism for reading the batched notification was broken in this PR to avoid making it unnecessarily large, the current PR solves this problem
To solve the problem we resave notifications on Maintenance with timestamp
now() + resave_time, thus removing the traffic jam from the notification queue in the form of constantly hitting notifications on Maintenance in the batchesIn addition, PR solves the problem with constant checking of notifications on Maintenance, now all checks will be done via
resave_timeCarefully select the value of the
resave_timevariable! You don't want to make it too large, as this will affect the delay in sending notifications on Maintenance