Use BigTable Columns as range keys#946
Conversation
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
|
I've already done a bunch of review on this, LGTM from me. |
gouthamve
force-pushed
the
bt-schema
branch
2 times, most recently
from
2eb86b1 to
f180c25
Compare
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
|
I think this needs to be rebased now that the index cache is in. Once that is done I will happily review it! |
|
I've updated the PR, but it's not reflecting because GH has issues rn: https://status.github.com/messages But you can put this PR back on your TODO now :) Thanks a lot for all the reviews! |
pkg/chunk/storage/factory.go
Outdated
| // NewStorageClient makes a storage client based on the configuration. | ||
| func NewStorageClient(cfg Config, schemaCfg chunk.SchemaConfig) (client chunk.StorageClient, err error) { | ||
| // Clients makes the storage clients based on the configuration. | ||
| func Clients(cfg Config, schemaCfg chunk.SchemaConfig) ([]chunk.StorageOpt, error) { |
There was a problem hiding this comment.
Feels like a bit of a name mismatch to have a method called Clients return StorageOpts
cmd/ruler/main.go
Outdated
| level.Error(util.Logger).Log("msg", "error initializing storage client", "err", err) | ||
| os.Exit(1) | ||
| } | ||
| schemaOpts := chunk.SchemaOpts(chunkStoreConfig, schemaConfig) |
There was a problem hiding this comment.
could this line be done inside chunk.NewStore since it looks like it is the same everywhere? Or is there a case you would want to use a NewStore with schemaOpts not generated like this?
|
|
||
| opts = append(opts, chunk.StorageOpt{From: model.Time(0), Client: client}) | ||
| if cfg.StorageClient == "gcp" && schemaCfg.BigtableColumnKeyFrom.IsSet() { | ||
| client, err = gcp.NewStorageClientColumnKey(context.Background(), cfg.GCPStorageConfig, schemaCfg) |
There was a problem hiding this comment.
Shouldn't this be a CachingStorageClient as well?
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
3 participants
This PR replaces #753, fixes #714.
Should improve performance for BigTable queries as we don't need to resend the row key multiple times.
Required a fair amount of refactoring of the composite store to support changing storage client based on time, as well as schema.