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

make boltdb shipper singleton and some other minor refactoring #1995

Merged

Conversation

sandeepsukhani
Copy link
Contributor

What this PR does / why we need it:
To avoid various problems that we can run into by allowing to run multiple boltdb shipper instance we are making it singleton.
This PR also includes the following minor refactorings:

  1. Renamed NewBoltDBIndexClient function to NewBoltDBIndexClientWithShipper.
  2. Accepting prometheus.Registerer in storage.NewStore so that it can be set to nil during tests otherwise tests would fail if it is called multiple times.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Documentation added
  • Tests updated

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@sandeepsukhani sandeepsukhani merged commit 21d4ca4 into grafana:master Apr 27, 2020
@shapeofarchitect
Copy link

shapeofarchitect commented Jun 23, 2020

@sandeepsukhani Great work on shipper . Sorry to comment on Merged request. May I ask if my understanding is correct , that with boltDB-shipper I can offload the indexes to object store (like Azure blob) . Also I am aware that chunks can already be stored in Azure blob. I am now trying to configure the indexes to be offloaded to azure via boltdb-shipper . I am using below config but it looks like not the right one :( .

I am trying to store chunks and Index in Azure Blob as a single object store for indexes and chunks.

config:
    chunk_store_config:
      max_look_back_period: 168h
    ingester:
      chunk_block_size: 262144
      chunk_idle_period: 4m
      chunk_retain_period: 2m
    table_manager:
      retention_deletes_enabled: true
      retention_period: 168h
    schema_config:
      configs:
        - from: 2018-04-15
          store: boltdb-shipper
          object_store: azure
          schema: v11
          index:
            prefix: loki_index_
            period: 168h
    storage_config:
      azure:
        container_name: chunks
        account_name: devloki
        account_key: xxxxxxxxxxxxxxxxxxx

      boltdb_shipper:
        active_index_directory: /loki/index
        shared_store: azure
        cache_location: /loki/boltdb-cache 

@sandeepsukhani
Copy link
Contributor Author

Yes, you are right about boltdb-shipper keeping the index in one of the supported object store.
The config looks right to me. If you are facing any problem can you please open a bug with more details like errors or unexpected behaviour from Loki?
Also, note that we are planning to enforce period config under index for boltdb-shipper to be 24h so you would have to either migrate the index using a tool that we would provide or drop that data if you don't care about it. Also please note that it is still experimental but we would start recommending to use it soon once we have other important upcoming changes merged, until then please use it only for keeping non-critical logs.

@shapeofarchitect
Copy link

Thank you very Much @sandeepsukhani. I met with an error today when tried to migrate to azure blob. I first created storage account and then applied Loki config above to use shipper and chunks to be stored in blob but hit the error below.

level=warn ts=2020-06-23T18:50:32.354922921Z caller=experimental.go:19 msg="experimental feature in use" feature="Azure Blob Storage"
level=error ts=2020-06-23T18:50:32.355151117Z caller=log.go:140 msg="error initialising loki" err="mkdir /loki/index: read-only file system\nerror creating index client\ngithub.com/cortexproject/cortex/pkg/chunk/storage.NewStore\n\t/src/loki/vendor/github.com/cortexproject/cortex/pkg/chunk/storage/factory.go:153\ngithub.com/grafana/loki/pkg/storage.NewStore\n\t/src/loki/pkg/storage/store.go:53\ngithub.com/grafana/loki/pkg/loki.(*Loki).initStore\n\t/src/loki/pkg/loki/modules.go:280\ngithub.com/grafana/loki/pkg/loki.(*Loki).initModuleServices\n\t/src/loki/pkg/loki/loki.go:192\ngithub.com/grafana/loki/pkg/loki.New\n\t/src/loki/pkg/loki/loki.go:134\nmain.main\n\t/src/loki/cmd/loki/main.go:76\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:203\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1357\nerror initialising module: store\ngithub.com/grafana/loki/pkg/loki.(*Loki).initModuleServices\n\t/src/loki/pkg/loki/loki.go:194\ngithub.com/grafana/loki/pkg/loki.New\n\t/src/loki/pkg/loki/loki.go:134\nmain.main\n\t/src/loki/cmd/loki/main.go:76\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:203\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1357"

@prateekkhera
Copy link

prateekkhera commented Jul 13, 2020

@shapeofarchitect , I am also getting the exact error, but for minio storage I configured. Did you manage to solve that?

@shapeofarchitect
Copy link

@prateekkhera so the error is a valid one because botdb-shipper component doesn't yet support the other cloud object stores. so I would say it's time for us to commit upstream the changes . This was my bad to commit on Merged Pull request . I guess we need to move off from this discussion and use the open issue and consider upstream pull request. I am trying to do one for Azure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants