Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 48 additions & 5 deletions transports/bifrost-http/lib/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -762,10 +762,28 @@ func registerFeatureFlags(_ context.Context) error {
// - Case conversion for provider names (e.g., "OpenAI" -> "openai")
// - In-memory storage for ultra-fast access during request processing
// - Graceful handling of missing config files
func LoadConfig(ctx context.Context, configDirPath string) (*Config, error) {
// LoadConfig loads configuration from configDirPath. logsDir overrides where logs.db is placed:
// by default (empty) logs.db is a sibling of config.db, which forces config and the large,
// churning request-log DB onto the same mount; a non-empty logsDir puts logs.db in its own
// directory so the two can be bind-mounted independently. The directory is set via the -logs-dir
// flag / BIFROST_LOGS_DIR env (see transports/bifrost-http/main.go) and only applies to the
// default SQLite logstore — when the logstore is explicitly configured (config.json) or non-SQLite,
// logsDir is ignored and a warning is logged.
func LoadConfig(ctx context.Context, configDirPath, logsDir string) (*Config, error) {
configFilePath := filepath.Join(configDirPath, "config.json")
configDBPath := filepath.Join(configDirPath, "config.db")
logsDBPath := filepath.Join(configDirPath, "logs.db")
if logsDir == "" {
logsDir = configDirPath
}
// Ensure the logs dir exists; an operator-supplied -logs-dir may not exist yet, and SQLite
// won't create the parent directory — it would fail to open logs.db and block startup.
if logsDir != configDirPath {
if err := os.MkdirAll(logsDir, 0o755); err != nil {
return nil, fmt.Errorf("failed to create logs directory %s: %w", logsDir, err)
}
}
logsDBPath := filepath.Join(logsDir, "logs.db")
logsDirOverridden := logsDir != configDirPath
// Initialize config
config := &Config{
configPath: configFilePath,
Expand Down Expand Up @@ -855,7 +873,7 @@ func LoadConfig(ctx context.Context, configDirPath string) (*Config, error) {
// 1a. Vault config acknowledgement (initialization handled by enterprise layer)
initVault(&configData)
// 2. Stores (config, logs, vector) — creates defaults for absent configs
if err := initStores(ctx, config, &configData, configDBPath, logsDBPath); err != nil {
if err := initStores(ctx, config, &configData, configDBPath, logsDBPath, logsDirOverridden); err != nil {
return nil, err
}
// 3. KV store
Expand Down Expand Up @@ -922,7 +940,7 @@ func LoadConfig(ctx context.Context, configDirPath string) (*Config, error) {

// initStores initializes config, logs, and vector stores.
// When config data sections are absent (nil), creates default SQLite stores for persistence.
func initStores(ctx context.Context, config *Config, configData *ConfigData, configDBPath, logsDBPath string) error {
func initStores(ctx context.Context, config *Config, configData *ConfigData, configDBPath, logsDBPath string, logsDirOverridden bool) error {
var err error
// Initialize config store
if configData.ConfigStoreConfig != nil && configData.ConfigStoreConfig.Enabled {
Expand Down Expand Up @@ -957,7 +975,11 @@ func initStores(ctx context.Context, config *Config, configData *ConfigData, con

// Initialize log store
if configData.LogsStoreConfig != nil && configData.LogsStoreConfig.Enabled {
// Explicit logs store configuration from config.json
// Explicit logs store configuration from config.json — it wins over the -logs-dir/env
// override, so warn rather than silently ignore the requested relocation.
if logsDirOverridden {
logger.Warn("logs dir override ignored: logs store is explicitly configured in config.json (-logs-dir / BIFROST_LOGS_DIR only applies to the default SQLite logs store)")
}
config.LogsStore, err = logstore.NewLogStore(ctx, configData.LogsStoreConfig, logger)
if err != nil {
return err
Expand All @@ -977,6 +999,27 @@ func initStores(ctx context.Context, config *Config, configData *ConfigData, con
return fmt.Errorf("failed to get logs store config: %w", dbErr)
}
}
// When a logs dir override is in effect, relocate the SQLite logs.db — but only when the
// stored path is the implicit default (next to config.db, or empty). A stored path that an
// operator explicitly configured takes precedence and is left unchanged (with a warning),
// so the override never silently switches an operator-chosen logs DB. logsDBPath already
// honors the override (see LoadConfig). Non-SQLite / unexpected payloads warn rather than
// silently no-op, so the operator isn't left wondering why logs.db didn't move.
defaultSQLiteLogsDBPath := filepath.Join(filepath.Dir(configDBPath), "logs.db")
if logsDirOverridden && logStoreConfig != nil {
if logStoreConfig.Type != logstore.LogStoreTypeSQLite {
logger.Warn("logs dir override ignored: stored logs store is %s, not SQLite (-logs-dir / BIFROST_LOGS_DIR only relocates the SQLite logs.db)", logStoreConfig.Type)
} else if sqliteConfig, ok := logStoreConfig.Config.(*logstore.SQLiteConfig); !ok {
logger.Warn("logs dir override ignored: stored SQLite logs store has an unexpected config payload; cannot relocate logs.db")
} else if sqliteConfig.Path == "" || sqliteConfig.Path == defaultSQLiteLogsDBPath {
if sqliteConfig.Path != logsDBPath {
logger.Info("logs dir override: relocating logs.db from stored path %s to %s", sqliteConfig.Path, logsDBPath)
sqliteConfig.Path = logsDBPath
}
} else if sqliteConfig.Path != logsDBPath {
logger.Warn("logs dir override ignored: stored SQLite path %s appears explicitly configured; keeping it unchanged", sqliteConfig.Path)
}
}
Comment on lines +1009 to +1022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 -logs-dir is silently one-shot after the first startup

On the first startup with -logs-dir /mnt/a, logStoreConfig is nil (nothing in DB yet), so the relocation block is skipped and a new config with Path: /mnt/a/logs.db is created and then persisted to config.db via UpdateLogsStoreConfig at line 1069. On the second startup with -logs-dir /mnt/b, GetLogsStoreConfig returns {Path: "/mnt/a/logs.db"}. The guard at line 1014 only permits relocation when the stored path is empty or equals defaultSQLiteLogsDBPath (i.e. $configDir/logs.db). Since /mnt/a/logs.db matches neither, control falls to line 1019 and logs a warning: "stored SQLite path appears explicitly configured; keeping it unchanged". The flag has no effect and /mnt/a/logs.db is persisted back again.

The code cannot distinguish "this path was written by a previous -logs-dir invocation" from "this path was explicitly set by the operator via the dashboard/API". The simplest fix is: when logsDirOverridden is true, always replace the stored path with logsDBPath — the flag is the operator's explicit runtime intent. Operators who want a pinned path via config.json are already protected by the explicit-config-wins branch earlier in this function. A subtest for "second startup with a changed logs dir" would catch this regression.

if logStoreConfig == nil {
logStoreConfig = &logstore.Config{
Enabled: true,
Expand Down
Loading