Skip to content

Backend Selector Feature#8

Open
ferranda wants to merge 3 commits intomainfrom
backend-selector-regex
Open

Backend Selector Feature#8
ferranda wants to merge 3 commits intomainfrom
backend-selector-regex

Conversation

@ferranda
Copy link
Copy Markdown
Collaborator

Motivation

We would like to select which backend carbonapi should query based on regex applied on queries.

for _, query := range queries {
realQueries = append(realQueries, *query)
}
defer wg.Done()
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should defer immediatly

close(statsRespCh)
close(errRespCh)
}()
res := <-queryRespCh
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wierd pattern. Should create a new MultiFetchResponse and call Append on it

close(statsRespCh)
close(errRespCh)
}()
res := <-queryRespCh
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dito

close(statsRespCh)
close(errRespCh)
}()
r := <-queryRespCh
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dito

Comment on lines +15 to +16
AllowGroupPerMetric := make(map[*protov3.FetchRequest][]string)
DenyGroupPerMetric := make(map[*protov3.FetchRequest][]string)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be private

queriesPerGroup[nameGroup] = append(queriesPerGroup[nameGroup], query)
}
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if there is no match? Shouldn't we broadcast or use a user supplied strategy?

return groupOfQueriesPerBackendServer
}

func contains(s []string, e string) bool {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strings.Contains

config.yml Outdated
keepAliveInterval: "30s"
maxIdleConnsPerHost: 100
backendsv2:
selector: "rb"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cryptic, prefer per_query or something meaningfull

config.yml Outdated
maxIdleConnsPerHost: 100
backendsv2:
selector: "rb"
groupRegexRules:
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectorRules

config.yml Outdated
backendsv2:
selector: "rb"
groupRegexRules:
- regex: "teadsWigoDebug_user_cookie_found_count"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

match

config.yml Outdated
selector: "rb"
groupRegexRules:
- regex: "teadsWigoDebug_user_cookie_found_count"
effect: "allow"
Copy link
Copy Markdown
Owner

@github-vincent-miszczak github-vincent-miszczak May 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

action: select|reject

config.yml Outdated
groupRegexRules:
- regex: "teadsWigoDebug_user_cookie_found_count"
effect: "allow"
groupBackendNames: ["prometheus"]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backends

for _, server := range backend.Servers {
config.Servers = []string{server}
config.GroupName = server
client, e := backendInit(q.logger, config, q.tldCacheDisabled)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why we need to create new client for each request. Can't we create clients at init and pick them on matches? TBD

func (r *RegexRulesResults) Fill(s string, rule RegexRuleGroupBackends) {
if rule.CompiledRegex.MatchString(s) {
if rule.Effect == "allow" {
r.Allow[s] = AppendUnique(r.Allow[s], rule.GroupBackendNames)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably better using map[string]map[string]bool

GroupRegexRules []RegexRuleGroupBackends `mapstructure:"groupRegexRules"`
}

type RegexRuleGroupBackends struct {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SelectionRule

@@ -0,0 +1,50 @@
package types

type RegexRulesResults struct {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SelectionRuleResults

}
}

func (r *RegexRulesResults) Fill(s string, rule RegexRuleGroupBackends) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s => query

zipper/zipper.go Outdated
zap.Any("merry.Errors", err),
)
var err merry.Error
if cfg.BackendsV2.Selector == "rb" || cfg.BackendsV2.Selector == "regexbased" {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer stacking storeClients

if cfg...enabled {
    storeClients = querydispatch.New(...,storeClients)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants