search

grafana / pyroscope

Continuous Profiling Platform. Debug performance issues down to a single line of code
https://grafana.com/oss/pyroscope/
GNU Affero General Public License v3.0
9.96k stars 595 forks source link

Figure how to run the store-gateway when using single-binary #2045

Open cyriltovena opened 1 year ago

cyriltovena commented 1 year ago

Right now the store gateway is not enable when using single binary, but we should definitively have it if you run using an cloud object storage.

simonswine commented 1 year ago

Just to add a bit more context: For different components we use the dskit/modules package, you can find the original design for Cortex (proper ancient).

The role of the store-gateway is very similar how it is explained in Grafana Mimir https://grafana.com/docs/mimir/latest/references/architecture/components/store-gateway/

Rustin170506 commented 1 year ago

I am working on this.

Rustin170506 commented 1 year ago

Filed a pull request: https://github.com/grafana/pyroscope/pull/2247

But I am not sure does it make sense, because it seems we don't think file-system is a valid storge.

Tested it locally. It didn't work.

level=error caller=phlare.go:390 ts=2023-08-14T12:48:27.894246Z msg="module failed" module=store-gateway error="invalid service state: Failed, expected: Running, failure: initial blocks synchronization: 2 errors: failed to synchronize Pyroscope blocks for user pyroscope: sync block: iter block metas: stat /Volumes/t7/code/pyroscope/test/pyroscope/phlaredb/01H79: stat /Volumes/t7/code/pyroscope/test/pyroscope/phlaredb/01H79: not a directory; failed to synchronize Pyroscope blocks for user demo.yaml: sync block: iter block metas: stat /Volumes/t7/code/pyroscope/test/demo.yaml/phlaredb/01H5T: stat /Volumes/t7/code/pyroscope/test/demo.yaml/phlaredb/01H5T: not a directory"
Rustin170506 commented 1 year ago

So the requirement is we only can enable it when we use cloud object storage with the single binary. Or do we need to change the file-system to support the store-gateway?

Rustin170506 commented 1 year ago

Oh, I guess the failure was caused by my env.

After I changed the config to:

# Do not use this configuration in production.
# It is for demonstration purposes only.
scrape_configs:
  - job_name: "default"
    scrape_interval: "15s"
    static_configs:
      - targets: ["127.0.0.1:4100"]
        labels:
          service_name: "service-demo"

# storage:
#   backend: s3
#   s3:
#     endpoint: 127.0.0.1:9000
#     bucket_name: test
#     insecure: true
#     access_key_id: minioadmin
#     secret_access_key: minioadmin

storage:
  backend: filesystem
  filesystem:
    dir: /tmp/pyroscope

Then it worked well.

simonswine commented 1 year ago

Looking at the potential combinations and what I would expect:

kolesnikovae commented 1 year ago
  • storage.backend not configured and target=all (single binary):
    • As right now we won't initialize the store-gatway and shipper.
  • storage.backend configured and target!=all (micro services):
    • Not valid we should error out, as the microservices need to have a shared storage between them

I was sure that the object storage should be an option rather than a requirement, regardless of how pyroscope is deployed.

Rustin170506 commented 1 year ago

storage.backend configured and target!=all (micro services):

  • Not valid we should error out, as the microservices need to have a shared storage between them

Does this is a typo? Should it be storage.backend not configured and target!=all (micro services)?

  • storage.backend not configured and target=all (single binary):

    • As right now we won't initialize the store-gatway and shipper.

  • storage.backend configured and target=all (single binary):

    • It is valid to use all type of storage.backend
    • Start the storage gateway and the shipper.

For Mimir, it always starts the storage gateway. https://github.com/grafana/mimir/blob/df45dbeeba4c5cd79a1ccec01fd24e19d933fb2a/pkg/mimir/modules.go#L974

So I think we can use filesystem as the default storage backend and start it. Then we don't require users to set real object storage, but they can still test or use the store gateway.

simonswine commented 1 year ago

Does this is a typo? Should it be storage.backend not configured and target!=all (micro services)?

Yes it was, sorry for the confusion.

I was sure that the object storage should be an option rather than a requirement, regardless of how pyroscope is deployed.

As soon as you run micro-services we require you to specify a storage that is shared between all micro-services, it could be a filesystem path on the same node or a bucket, but it is not valid to not specify a storage and only run on ingester local blocks. (This is also the status quo, try running a component it will fail until you configure an object store).

For Mimir, it always starts the storage gateway [...]

Yes true, but for us the ingester already allows local blocks to be queried, the only benefit would be in specifying a directory that is shared by all ingesters/store-gateways, which we can't default to a filesystem path, because e.g. two components might run on different nodes.

Rustin170506 commented 1 year ago

Yes true, but for us the ingester already allows local blocks to be queried, the only benefit would be in specifying a directory that is shared by all ingesters/store-gateways, which we can't default to a filesystem path, because e.g. two components might run on different nodes.

Sounds readable to me.

cyriltovena commented 1 year ago

Store gateway with filesystem won't work see https://github.com/grafana/pyroscope/issues/2029 this needs to be solved before activating store-gateway with filesystem.

but I think we can still go ahead with cloud storage though

simonswine commented 1 year ago

Not fixed