Skip to content

Remove shared_store and shared_store_key_prefix from shipper and compactor #10840

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

Merged
merged 20 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
misc corrections
  • Loading branch information
ashwanthgoli committed Oct 11, 2023
commit 727222523a8c0d0c72cc0e72793bf8c6952bc2d7
2 changes: 1 addition & 1 deletion docs/sources/configure/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -4115,7 +4115,7 @@ The `period_config` block configures what index schemas should be used for from
index:
# Path prefix for index tables. Prefix always needs to end with a path
# delimiter '/', except when the prefix is empty.
[path_prefix: <string> | default = "/index"]
[path_prefix: <string> | default = "index/"]

# Table prefix for all period tables.
[prefix: <string> | default = ""]
Expand Down
1 change: 0 additions & 1 deletion pkg/logcli/query/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,6 @@ func TestLoadFromURL(t *testing.T) {
},
}

// Missing SharedStoreType should error
cm := storage.NewClientMetrics()
client, err := GetObjectClient(config.StorageTypeFileSystem, conf, cm)
require.NoError(t, err)
Expand Down
48 changes: 28 additions & 20 deletions pkg/loki/config_wrapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -737,13 +737,13 @@ compactor:
})
})

t.Run("when using compactor storage type", func(t *testing.T) {
t.Run("shared store types provided via config file take precedence", func(t *testing.T) {
const boltdbSchemaConfig = `---
t.Run("compactor shared storage type", func(t *testing.T) {
t.Run("shared store types provided via config file", func(t *testing.T) {
const schemaConfig = `---
schema_config:
configs:
- from: 2021-08-01
store: boltdb-shipper
store: tsdb
object_store: gcs
schema: v11
index:
Expand All @@ -752,29 +752,35 @@ schema_config:

compactor:
shared_store: s3`
cfg, _ := testContext(boltdbSchemaConfig, nil)
cfg, _ := testContext(schemaConfig, nil)

assert.Equal(t, config.StorageTypeS3, cfg.CompactorConfig.SharedStoreType)
})

t.Run("shared store types provided via command line take precedence", func(t *testing.T) {
const boltdbSchemaConfig = `---
const schemaConfig = `---
schema_config:
configs:
- from: 2021-08-01
store: boltdb-shipper
store: tsdb
object_store: gcs
schema: v11
index:
prefix: index_
period: 24h`
cfg, _ := testContext(boltdbSchemaConfig, []string{"-boltdb.shipper.compactor.shared-store", "s3"})
period: 24h

compactor:
shared_store: gcs`
cfg, _ := testContext(schemaConfig, []string{"-compactor.shared-store", "s3"})

assert.Equal(t, config.StorageTypeS3, cfg.CompactorConfig.SharedStoreType)
})
})

t.Run("boltdb shipper apply common path prefix", func(t *testing.T) {
t.Run("if path prefix provided in common config, default active_index_directory and cache_location", func(t *testing.T) {

t.Run("if path prefix provided in common config, default active_index_directory and cache_location", func(t *testing.T) {})
const boltdbSchemaConfig = `---
const boltdbSchemaConfig = `---
common:
path_prefix: /opt/loki
schema_config:
Expand All @@ -786,14 +792,14 @@ schema_config:
index:
prefix: index_
period: 24h`
config, _ := testContext(boltdbSchemaConfig, []string{"-boltdb.shipper.compactor.shared-store", "s3"})
config, _ := testContext(boltdbSchemaConfig, nil)

assert.Equal(t, "/opt/loki/boltdb-shipper-active", config.StorageConfig.BoltDBShipperConfig.ActiveIndexDirectory)
assert.Equal(t, "/opt/loki/boltdb-shipper-cache", config.StorageConfig.BoltDBShipperConfig.CacheLocation)
})
assert.Equal(t, "/opt/loki/boltdb-shipper-active", config.StorageConfig.BoltDBShipperConfig.ActiveIndexDirectory)
assert.Equal(t, "/opt/loki/boltdb-shipper-cache", config.StorageConfig.BoltDBShipperConfig.CacheLocation)
})

t.Run("boltdb shipper directories correctly handle trailing slash in path prefix", func(t *testing.T) {
const boltdbSchemaConfig = `---
t.Run("boltdb shipper directories correctly handle trailing slash in path prefix", func(t *testing.T) {
const boltdbSchemaConfig = `---
common:
path_prefix: /opt/loki/
schema_config:
Expand All @@ -805,10 +811,12 @@ schema_config:
index:
prefix: index_
period: 24h`
config, _ := testContext(boltdbSchemaConfig, []string{"-boltdb.shipper.compactor.shared-store", "s3"})
config, _ := testContext(boltdbSchemaConfig, nil)

assert.Equal(t, "/opt/loki/boltdb-shipper-active", config.StorageConfig.BoltDBShipperConfig.ActiveIndexDirectory)
assert.Equal(t, "/opt/loki/boltdb-shipper-cache", config.StorageConfig.BoltDBShipperConfig.CacheLocation)
assert.Equal(t, "/opt/loki/boltdb-shipper-active", config.StorageConfig.BoltDBShipperConfig.ActiveIndexDirectory)
assert.Equal(t, "/opt/loki/boltdb-shipper-cache", config.StorageConfig.BoltDBShipperConfig.CacheLocation)

})
})

t.Run("ingester final sleep config", func(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions pkg/storage/config/schema_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func (cfg *SchemaConfig) Validate() error {
periodCfg := &cfg.Configs[i]
periodCfg.applyDefaults()
if err := periodCfg.validate(); err != nil {
return err
return fmt.Errorf("validating period_config: %w", err)
}

if i+1 < len(cfg.Configs) {
Expand Down Expand Up @@ -423,7 +423,7 @@ func (cfg PeriodConfig) validate() error {
}

if err := cfg.IndexTables.Validate(); err != nil {
return err
return fmt.Errorf("validating index: %w", err)
}

if cfg.ChunkTables.Period > 0 && cfg.ChunkTables.Period%(24*time.Hour) != 0 {
Expand Down Expand Up @@ -478,7 +478,7 @@ func (cfg *PeriodConfig) VersionAsInt() (int, error) {
}

type IndexPeriodicTableConfig struct {
PathPrefix string `yaml:"path_prefix" doc:"default=/index|description=Path prefix for index tables. Prefix always needs to end with a path delimiter '/', except when the prefix is empty."`
PathPrefix string `yaml:"path_prefix" doc:"default=index/|description=Path prefix for index tables. Prefix always needs to end with a path delimiter '/', except when the prefix is empty."`
PeriodicTableConfig `yaml:",inline"`
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/storage/config/schema_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ func TestSchemaConfig_Validate(t *testing.T) {

t.Run(testName, func(t *testing.T) {
actual := testData.config.Validate()
assert.Equal(t, testData.err, actual)
assert.ErrorIs(t, actual, testData.err)
if testData.expected != nil {
require.Equal(t, testData.expected, testData.config)
}
Expand Down Expand Up @@ -489,7 +489,7 @@ func TestPeriodConfig_Validate(t *testing.T) {
if tc.err == "" {
require.Nil(t, tc.in.validate())
} else {
require.Error(t, tc.in.validate(), tc.err)
require.ErrorContains(t, tc.in.validate(), tc.err)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion tools/tsdb/helpers/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func getTableNumberForTime(t model.Time) int64 {
func GetPeriodConfigForTableNumber(table string, periodicConfigs []config.PeriodConfig) (config.PeriodConfig, config.TableRange, string, error) {
tableNo, err := extractTableNumberFromName(table)
if err != nil {
return config.PeriodConfig{}, config.TableRange{}, "", err
return config.PeriodConfig{}, config.TableRange{}, "", fmt.Errorf("extracting table number: %w", err)
}

for i, periodCfg := range periodicConfigs {
Expand Down
2 changes: 1 addition & 1 deletion tools/tsdb/index-analyzer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,6 @@ func main() {
tenants, err := helpers.ResolveTenants(objectClient, periodCfg.IndexTables.PathPrefix, tableName)
helpers.ExitErr("resolving tenants", err)

err = analyze(shipper, bucket, tenants)
err = analyze(shipper, tableName, tenants)
helpers.ExitErr("analyzing", err)
}