Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: take pager TTL from config instead of hardcoded const #1139

Merged
merged 9 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,20 @@ func (WebConfig) Render(http.ResponseWriter, *http.Request) error {
const (
// DefaultTriggerNameMaxSize which will be used while validating dto.Trigger.
DefaultTriggerNameMaxSize = 200
// DefaultTriggerPagerTTL which will be used via trigger creation.
DefaultTriggerPagerTTL = time.Minute * 30
)

// PagerLimits contains all limits applied for pagers.
type PagerLimits struct {
// TTL is the amount of time that the pager will be exist
TTL time.Duration
}

// LimitsConfig contains limits for some entities.
type LimitsConfig struct {
// Pager contains limits for pagers.
Pager PagerLimits
// Trigger contains limits for triggers.
Trigger TriggerLimits
// Trigger contains limits for teams.
Expand All @@ -102,6 +112,9 @@ type TriggerLimits struct {
// GetTestLimitsConfig is used for testing.
func GetTestLimitsConfig() LimitsConfig {
return LimitsConfig{
Pager: PagerLimits{
TTL: DefaultTriggerPagerTTL,
},
Trigger: TriggerLimits{
MaxNameSize: DefaultTriggerNameMaxSize,
},
Expand Down
2 changes: 1 addition & 1 deletion api/controller/triggers.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func SearchTriggers(database moira.Database, searcher moira.Searcher, options mo
return nil, api.ErrorInternalServer(err)
}
options.PagerID = uuid4.String()
err = database.SaveTriggersSearchResults(options.PagerID, searchResults)
err = database.SaveTriggersSearchResults(options.PagerID, searchResults, options.PagerTTL)
if err != nil {
return nil, api.ErrorInternalServer(err)
}
Expand Down
4 changes: 3 additions & 1 deletion api/controller/triggers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"testing"
"time"

"github.com/gofrs/uuid"
"github.com/moira-alert/moira"
Expand Down Expand Up @@ -604,10 +605,11 @@ func TestSearchTriggers(t *testing.T) {
searchOptions.Page = 0
searchOptions.Size = -1
searchOptions.CreatePager = true
searchOptions.PagerTTL = time.Hour * 2
exp = 31
gomock.InOrder(
mockIndex.EXPECT().SearchTriggers(searchOptions).Return(triggerSearchResults, exp, nil),
mockDatabase.EXPECT().SaveTriggersSearchResults(gomock.Any(), triggerSearchResults).Return(nil).Do(func(pID string, _ interface{}) {
mockDatabase.EXPECT().SaveTriggersSearchResults(gomock.Any(), triggerSearchResults, gomock.Any()).Return(nil).Do(func(pID string, _ interface{}, _ interface{}) {
searchOptions.PagerID = pID
}),
mockDatabase.EXPECT().GetTriggerChecks(triggerIDs).Return(triggersPointers, nil),
Expand Down
3 changes: 2 additions & 1 deletion api/handler/triggers.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func triggers(metricSourceProvider *metricSource.SourceProvider, searcher moira.
router.Route("/{triggerId}", trigger)
router.With(middleware.Paginate(0, 10)).With(middleware.Pager(false, "")).Get("/search", searchTriggers)
router.With(middleware.Pager(false, "")).Delete("/search/pager", deletePager)
// ToDo: DEPRECATED method. Remove in Moira 2.6
// TODO: DEPRECATED method. Remove in Moira 2.6
router.With(middleware.Paginate(0, 10)).With(middleware.Pager(false, "")).Get("/page", searchTriggers)
}
}
Expand Down Expand Up @@ -330,6 +330,7 @@ func searchTriggers(writer http.ResponseWriter, request *http.Request) {
NeedSearchByCreatedBy: ok,
CreatePager: middleware.GetCreatePager(request),
PagerID: middleware.GetPagerID(request),
PagerTTL: middleware.GetLimits(request).Pager.TTL,
}

triggersList, errorResponse := controller.SearchTriggers(database, searchIndex, searchOptions)
Expand Down
14 changes: 14 additions & 0 deletions cmd/api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,20 @@ type apiConfig struct {

// LimitsConfig contains configurable moira limits.
type LimitsConfig struct {
// Pager contains the limits applied to pagers.
Pager PagerLimits `yaml:"pager"`
// Trigger contains the limits applied to triggers.
Trigger TriggerLimitsConfig `yaml:"trigger"`
// Team contains the limits applied to teams.
Team TeamLimitsConfig `yaml:"team"`
}

// PagerLimits represents the limits which will be applied to all pagers.
type PagerLimits struct {
// TTL is the amount of time that the pager will be exist
TTL time.Duration `yaml:"ttl"`
}

// TriggerLimitsConfig represents the limits which will be applied to all triggers.
type TriggerLimitsConfig struct {
// MaxNameSize is the max amount of characters allowed in trigger name.
Expand All @@ -77,6 +85,9 @@ type TeamLimitsConfig struct {
// ToLimits converts LimitsConfig to api.LimitsConfig.
func (conf LimitsConfig) ToLimits() api.LimitsConfig {
return api.LimitsConfig{
Pager: api.PagerLimits{
TTL: conf.Pager.TTL,
},
Trigger: api.TriggerLimits{
MaxNameSize: conf.Trigger.MaxNameSize,
},
Expand Down Expand Up @@ -281,6 +292,9 @@ func getDefault() config {
Listen: ":8081",
EnableCORS: false,
Limits: LimitsConfig{
Pager: PagerLimits{
TTL: api.DefaultTriggerPagerTTL,
},
Trigger: TriggerLimitsConfig{
MaxNameSize: api.DefaultTriggerNameMaxSize,
},
Expand Down
3 changes: 3 additions & 0 deletions cmd/api/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ func Test_webConfig_getDefault(t *testing.T) {
Listen: ":8081",
EnableCORS: false,
Limits: LimitsConfig{
Pager: PagerLimits{
TTL: api.DefaultTriggerPagerTTL,
},
Trigger: TriggerLimitsConfig{
MaxNameSize: api.DefaultTriggerNameMaxSize,
},
Expand Down
6 changes: 2 additions & 4 deletions database/redis/triggers_search_results.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import (
"github.com/moira-alert/moira/database/redis/reply"
)

const triggersSearchResultsExpire = time.Second * 1800

// SaveTriggersSearchResults is a function that takes an ID of pager and saves it to redis.
func (connector *DbConnector) SaveTriggersSearchResults(searchResultsID string, searchResults []*moira.SearchResult) error {
func (connector *DbConnector) SaveTriggersSearchResults(searchResultsID string, searchResults []*moira.SearchResult, recordTTL time.Duration) error {
ctx := connector.context
pipe := (*connector.client).TxPipeline()

Expand All @@ -27,7 +25,7 @@ func (connector *DbConnector) SaveTriggersSearchResults(searchResultsID string,
return fmt.Errorf("failed to PUSH: %w", err)
}
}
if err := pipe.Expire(ctx, resultsID, triggersSearchResultsExpire).Err(); err != nil {
if err := pipe.Expire(ctx, resultsID, recordTTL).Err(); err != nil {
return fmt.Errorf("failed to set expire time: %w", err)
}
response, err := pipe.Exec(ctx)
Expand Down
5 changes: 3 additions & 2 deletions database/redis/triggers_search_results_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package redis
import (
"fmt"
"testing"
"time"

"github.com/moira-alert/moira"
logging "github.com/moira-alert/moira/logging/zerolog_adapter"
Expand Down Expand Up @@ -96,7 +97,7 @@ func TestTriggersSearchResultsStoring(t *testing.T) {
defer dataBase.Flush()

Convey("Search Results Manipulation", t, func() {
err := dataBase.SaveTriggersSearchResults(searchResultsID, searchResults)
err := dataBase.SaveTriggersSearchResults(searchResultsID, searchResults, time.Hour)
So(err, ShouldBeNil)

actual, total, err := dataBase.GetTriggersSearchResults(searchResultsID, 0, -1)
Expand Down Expand Up @@ -176,7 +177,7 @@ func BenchmarkSaveTriggersSearchResults(b *testing.B) {
dataBase.Flush()
b.Run(fmt.Sprintf("Benchmark%d", limit), func(b *testing.B) {
for n := 0; n < b.N; n++ {
dataBase.SaveTriggersSearchResults(fmt.Sprintf("test_%d_%d", limit, n), data) //nolint
dataBase.SaveTriggersSearchResults(fmt.Sprintf("test_%d_%d", limit, n), data, time.Hour) //nolint
}
})
}
Expand Down
1 change: 1 addition & 0 deletions datatypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ type SearchOptions struct {
NeedSearchByCreatedBy bool
CreatePager bool
PagerID string
PagerTTL time.Duration
}

// MaintenanceCheck set maintenance user, time.
Expand Down
2 changes: 1 addition & 1 deletion interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type Database interface {

// SearchResult AKA pager storing
GetTriggersSearchResults(searchResultsID string, page, size int64) ([]*SearchResult, int64, error)
SaveTriggersSearchResults(searchResultsID string, searchResults []*SearchResult) error
SaveTriggersSearchResults(searchResultsID string, searchResults []*SearchResult, recordTTL time.Duration) error
IsTriggersSearchResultsExist(pagerID string) (bool, error)
DeleteTriggersSearchResults(pagerID string) error

Expand Down
8 changes: 4 additions & 4 deletions mock/moira-alert/database.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading