Skip to content

Commit

Permalink
fix(blooms): Fix panic in initialisation of the bloom planner and bui…
Browse files Browse the repository at this point in the history
…lder (#14110)

The targets `bloom-planner` and `bloom-builder` panic at startup on branch `main`.

```console
$ make loki && ./cmd/loki/loki -target=bloom-builder
CGO_ENABLED=0 go build -ldflags "-extldflags \"-static\" -s -w -X github.com/grafana/loki/v3/pkg/util/build.Branch=main -X github.com/grafana/loki/v3/pkg/util/build.Version=main-b29b4b4 -X github.com/grafana/loki/v3/pkg/util/build.Revision=b29b4b4bb -X github.com/grafana/loki/v3/pkg/util/build.BuildUser=christian@grafana1210 -X github.com/grafana/loki/v3/pkg/util/build.BuildDate=2024-09-11T08:30:11Z" -tags netgo -o cmd/loki/loki ./cmd/loki
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x287b590]

goroutine 1 [running]:
github.com/grafana/dskit/ring.(*BasicLifecycler).GetInstanceID(...)
	/home/christian/sandbox/grafana/loki/vendor/github.com/grafana/dskit/ring/basic_lifecycler.go:132
github.com/grafana/loki/v3/pkg/bloombuild/builder.New({{0x6400000, 0x6400000, {0x0, 0x0}, 0x0, 0x0, 0x0, {0x5f5e100, 0x2540be400, 0xa}, ...}, ...}, ...)
	/home/christian/sandbox/grafana/loki/pkg/bloombuild/builder/builder.go:92 +0x470
github.com/grafana/loki/v3/pkg/loki.(*Loki).initBloomBuilder(0xc002984008)
	/home/christian/sandbox/grafana/loki/pkg/loki/modules.go:1750 +0x4b6
github.com/grafana/dskit/modules.(*Manager).initModule(0xc000d2e6f0, {0x7ffe36edcb45, 0xd}, 0xc0027a22f8, 0xc0013ce960)
	/home/christian/sandbox/grafana/loki/vendor/github.com/grafana/dskit/modules/modules.go:136 +0x1ea
github.com/grafana/dskit/modules.(*Manager).InitModuleServices(0xc000d2e6f0, {0xc000dbeed0, 0x1, 0x7510c18f88e2e5ce?})
	/home/christian/sandbox/grafana/loki/vendor/github.com/grafana/dskit/modules/modules.go:108 +0xe8
github.com/grafana/loki/v3/pkg/loki.(*Loki).Run(0xc002984008, {0x0?, {0x4?, 0x2?, 0x6457fc0?}})
	/home/christian/sandbox/grafana/loki/pkg/loki/loki.go:497 +0x97
main.main()
	/home/christian/sandbox/grafana/loki/cmd/loki/main.go:129 +0x131e
````

This is because `t.indexGatewayRingManager` is not nil and therefore incorrectly tries to initialize the ring watcher, which fails.

---
Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
  • Loading branch information
chaudum authored Sep 11, 2024
1 parent 4d274f0 commit 8307c42
Showing 1 changed file with 14 additions and 10 deletions.
24 changes: 14 additions & 10 deletions pkg/loki/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -1709,8 +1709,14 @@ func (t *Loki) initBloomPlanner() (services.Service, error) {

logger := log.With(util_log.Logger, "component", "bloom-planner")

var ringManager *lokiring.RingManager
if t.Cfg.isTarget(Backend) && t.indexGatewayRingManager != nil {
// Bloom planner and builder are part of the backend target in Simple Scalable Deployment mode.
// To avoid creating a new ring just for this special case, we can use the index gateway ring, which is already
// part of the backend target. The planner creates a watcher service that regularly checks which replica is
// the leader. Only the leader plans the tasks. Builders connect to the leader instance to pull tasks.
level.Info(logger).Log("msg", "initializing bloom planner in ring mode as part of backend target")
ringManager = t.indexGatewayRingManager
}

p, err := planner.New(
Expand All @@ -1722,11 +1728,7 @@ func (t *Loki) initBloomPlanner() (services.Service, error) {
t.BloomStore,
logger,
prometheus.DefaultRegisterer,
// Bloom planner and builder are part of the backend target in Simple Scalable Deployment mode.
// To avoid creating a new ring just for this special case, we can use the index gateway ring, which is already
// part of the backend target. The planner creates a watcher service that regularly checks which replica is
// the leader. Only the leader plans the tasks. Builders connect to the leader instance to pull tasks.
t.indexGatewayRingManager,
ringManager,
)
if err != nil {
return nil, err
Expand All @@ -1743,8 +1745,14 @@ func (t *Loki) initBloomBuilder() (services.Service, error) {

logger := log.With(util_log.Logger, "component", "bloom-builder")

var ringManager *lokiring.RingManager
if t.Cfg.isTarget(Backend) && t.indexGatewayRingManager != nil {
// Bloom planner and builder are part of the backend target in Simple Scalable Deployment mode.
// To avoid creating a new ring just for this special case, we can use the index gateway ring, which is already
// part of the backend target. The planner creates a watcher service that regularly checks which replica is
// the leader. Only the leader plans the tasks. Builders connect to the leader instance to pull tasks.
level.Info(logger).Log("msg", "initializing bloom builder in ring mode as part of backend target")
ringManager = t.indexGatewayRingManager
}

return builder.New(
Expand All @@ -1757,11 +1765,7 @@ func (t *Loki) initBloomBuilder() (services.Service, error) {
t.BloomStore,
logger,
prometheus.DefaultRegisterer,
// Bloom planner and builder are part of the backend target in Simple Scalable Deployment mode.
// To avoid creating a new ring just for this special case, we can use the index gateway ring, which is already
// part of the backend target. The planner creates a watcher service that regularly checks which replica is
// the leader. Only the leader plans the tasks. Builders connect to the leader instance to pull tasks.
t.indexGatewayRingManager,
ringManager,
)
}

Expand Down

0 comments on commit 8307c42

Please sign in to comment.