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

Loki: Override distributor's default ring KV store #4440

Merged
merged 3 commits into from
Oct 13, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## Main
* [4400](https://github.com/grafana/loki/pull/4400) **trevorwhitney**: Config: automatically apply memberlist config too all rings when provided
* [4435](https://github.com/grafana/loki/pull/4435) **trevorwhitney**: Change default values for two GRPC settings so querier can connect to frontend/scheduler
* [4440](https://github.com/grafana/loki/pull/4440) **DylanGuedes**: Config: Override distributor's default ring KV store
* [4443](https://github.com/grafana/loki/pull/4443) **DylanGuedes**: Loki: Change how push API checks for contentType

# 2.3.0 (2021/08/06)
Expand Down
16 changes: 15 additions & 1 deletion docs/sources/upgrading/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@ If possible try to stay current and do sequential updates. If you want to skip v

### Loki

#### Distributor now stores ring in memory by default instead of Consul

PR [4440](https://github.com/grafana/loki/pull/4440) **DylanGuedes**: Config: Override distributor's default ring KV store

This change sets `inmemory` as the new default storage for the Distributor ring (previously `consul`).
The motivation is making the Distributor easier to run with default configs, by not requiring Consul anymore.
In any case, if you prefer to use Consul as the ring storage, you can set it by using the following config:

```yaml
distributor:
ring:
kvstore:
store: consul
```

#### Memberlist config now automatically applies to all non-configured rings
PR [4400](https://github.com/grafana/loki/pull/4400) **trevorwhitney**: Config: automatically apply memberlist config too all rings when provided

Expand Down Expand Up @@ -84,7 +99,6 @@ Please manually provide the values of `5m` and `true` (respectively) in your con

-_add changes here which are unreleased_


## 2.3.0

### Loki
Expand Down
23 changes: 20 additions & 3 deletions pkg/distributor/distributor.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,26 @@ type Config struct {
factory ring_client.PoolFactory `yaml:"-"`
}

// RegisterFlags registers the flags.
func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
cfg.DistributorRing.RegisterFlags(f)
// RegisterFlags registers distributor-related flags.
//
// Since they are registered through an external library, we override some of them to set
// different default values.
func (cfg *Config) RegisterFlags(fs *flag.FlagSet) {
throwaway := flag.NewFlagSet("throwaway", flag.PanicOnError)

cfg.DistributorRing.RegisterFlags(throwaway)

// Register to throwaway flags first. Default values are remembered during registration and cannot be changed,
// but we can take values from throwaway flag set and reregister into supplied flags with new default values.
throwaway.VisitAll(func(f *flag.Flag) {
// Ignore errors when setting new values. We have a test to verify that it works.
switch f.Name {
case "distributor.ring.store":
_ = f.Value.Set("inmemory")
}

fs.Var(f.Value, f.Name, f.Usage)
})
}

// Distributor coordinates replicates and distribution of log streams.
Expand Down
41 changes: 21 additions & 20 deletions pkg/loki/loki_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand All @@ -25,34 +24,36 @@ func TestFlagDefaults(t *testing.T) {

const delim = '\n'

minTimeChecked := false
pingWithoutStreamChecked := false
// Populate map with parsed default flags.
// Key is the flag and value is the default text.
gotFlags := make(map[string]string)
for {
line, err := buf.ReadString(delim)
if err == io.EOF {
break
}

require.NoError(t, err)

if strings.Contains(line, "-server.grpc.keepalive.min-time-between-pings") {
nextLine, err := buf.ReadString(delim)
require.NoError(t, err)
assert.Contains(t, nextLine, "(default 10s)")
minTimeChecked = true
}
nextLine, err := buf.ReadString(delim)
require.NoError(t, err)

if strings.Contains(line, "-server.grpc.keepalive.ping-without-stream-allowed") {
nextLine, err := buf.ReadString(delim)
require.NoError(t, err)
assert.Contains(t, nextLine, "(default true)")
pingWithoutStreamChecked = true
}
trimmedLine := strings.Trim(line, " \n")
splittedLine := strings.Split(trimmedLine, " ")[0]
gotFlags[splittedLine] = nextLine
}

require.True(t, minTimeChecked)
require.True(t, pingWithoutStreamChecked)
flagToCheck := "-distributor.ring.store"
require.Contains(t, gotFlags, flagToCheck)
require.Equal(t, c.Distributor.DistributorRing.KVStore.Store, "inmemory")
require.Contains(t, gotFlags[flagToCheck], "(default \"inmemory\")")

flagToCheck = "-server.grpc.keepalive.min-time-between-pings"
require.Contains(t, gotFlags, flagToCheck)
require.Equal(t, c.Server.GRPCServerMinTimeBetweenPings, 10*time.Second)
require.Contains(t, gotFlags[flagToCheck], "(default 10s)")

require.Equal(t, true, c.Server.GRPCServerPingWithoutStreamAllowed)
require.Equal(t, 10*time.Second, c.Server.GRPCServerMinTimeBetweenPings)
flagToCheck = "-server.grpc.keepalive.ping-without-stream-allowed"
require.Contains(t, gotFlags, flagToCheck)
require.Equal(t, c.Server.GRPCServerPingWithoutStreamAllowed, true)
require.Contains(t, gotFlags[flagToCheck], "(default true)")
}