Support 0 for model.Duration in the table manager

[cyriltovena]

Recently the table manager period has been changed to use model.Duration from Prometheus because of a vendoring in Cortex. see https://github.com/grafana/loki/blob/master/vendor/github.com/cortexproject/cortex/pkg/chunk/table_manager.go#L95 and #1838

The problem is that before time.Duration was supporting the value 0. Before it propagates to other fields we should try to suggest in the prometheus common project a change to support the 0 value. Then backport this to Cortex and Loki.

[cyriltovena]

PR to Prometheus. prometheus/common#226 let's see.

[cyriltovena]

Prometheus won't accept this PR. I don't think it's worth wasting time arguing or pushing further. We'll have to use the new 0s default.

[cyriltovena]

Change of plan upstream Prometheus accepted the PR. We need to update cortex and then get that into Loki. Should be easy for anyone to take it.

[mmedum]

Hi @cyriltovena i would like to do some work on this. Could you get me up to speed quickly on this? :)

[cyriltovena]

Sure:

• First you need to update prometheus common in cortex. using Support 0 for model.Duration. prometheus/common#226
• Then update Loki from the latest cortex.
• That's should be it really it will support the 0 duration now.

Maybe you should wait for #1869 before starting anything so you don't get conflicts.

[mmedum]

Will do. 🙂

Just going to start on one of the other issues.

[mmedum]

Hi again @cyriltovena

From what I can tell, PRs should be merged for dependencies. Are you still missing this update or did it already get merged with resolution of #1869?

[cyriltovena]

I need to check

[lukashass]

I just came across an error that I suppose would be fixed by resolving this issue.

failed parsing config: /etc/loki/local-config.yaml: not a valid duration string: "0" while using grafana/loki:latest with the default config file.

It seems this line still uses 0 while the last line in the file was changed to using 0s in 0ad1c6c.

[cyriltovena]

@mmedum This is still an issue and this hasn't been fixed by #1869.

If you want to take this, first open a PR on cortex to update prometheus/common there to latest.

Once it's merged in cortex, open a PR here to update cortex to latest.

Good luck ✌️

@lukashass you're probably hitting another one.

chunk_store_config:
  max_look_back_period: 0s

Use this instead of the default and the error will go away.

[mmedum]

@cyriltovena Thanks.

But maybe I am a bit useless, from what I can see, cortexproject/cortex uses prometheus/common lastest already.

I have in my fork updated loki to use cortexproject/cortex v1.0.0, as that is the lastest release from what I can tell. But it seems I am hitting a snag with:

go: finding module for package github.com/cortexproject/cortex/pkg/ring/client
github.com/grafana/loki/pkg/distributor imports
	github.com/cortexproject/cortex/pkg/ring/client: module github.com/cortexproject/cortex@latest found (v1.0.0), but does not contain package github.com/cortexproject/cortex/pkg/ring/client

[cyriltovena]

Let me check

[cyriltovena]

So Cortex doesn't use latest prometheus/common, you can either try to overrides in Loki or update it first in Cortex.

[ghostsquad]

I'm following the Tanka installation guide, and I'm running into the same problem. What keys in the jsonnet need to be updated to be 0s instead of 0 ?

[ghostsquad]

I think I resolved with this below. It would be nice if the defaults just worked.

  _config+:: {
    ...
    loki+: {
      chunk_store_config+: {
        max_look_back_period: "0s",
      },
      table_manager+: {
        retention_period: "0s",
      },
    },
  },