From c7d17479a003200aa6bd5122bb8e718d56186b15 Mon Sep 17 00:00:00 2001 From: Nir Ozery Date: Tue, 25 Feb 2025 11:34:53 -0500 Subject: [PATCH 1/6] Graveler populate storage ID --- pkg/catalog/catalog.go | 4 +++- pkg/config/config.go | 5 +++++ pkg/graveler/ref/manager.go | 9 ++++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/catalog/catalog.go b/pkg/catalog/catalog.go index bb2fab0e0fa..8db68c541d9 100644 --- a/pkg/catalog/catalog.go +++ b/pkg/catalog/catalog.go @@ -353,6 +353,7 @@ func New(ctx context.Context, cfg Config) (*Catalog, error) { storeLimiter := kv.NewStoreLimiter(cfg.KVStore, limiter) addressProvider := ident.NewHexAddressProvider() + refManager := ref.NewRefManager( ref.ManagerConfig{ Executor: executor, @@ -363,7 +364,8 @@ func New(ctx context.Context, cfg Config) (*Catalog, error) { CommitCacheConfig: ref.CacheConfig(baseCfg.Graveler.CommitCache), MaxBatchDelay: baseCfg.Graveler.MaxBatchDelay, BranchApproximateOwnershipParams: makeBranchApproximateOwnershipParams(baseCfg.Graveler.BranchOwnership), - }) + }, cfg.Config.StorageConfig(), + ) gcManager := retention.NewGarbageCollectionManager(tierFSParams.Adapter, refManager, baseCfg.Committed.BlockStoragePrefix) settingManager := settings.NewManager(refManager, cfg.KVStore) if cfg.SettingsManagerOption != nil { diff --git a/pkg/config/config.go b/pkg/config/config.go index b0070a03ccb..0ac0af2261a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -163,6 +163,7 @@ type AdapterConfig interface { BlockstoreAzureParams() (blockparams.Azure, error) GetDefaultNamespacePrefix() *string IsBackwardsCompatible() bool + ID() string } type Blockstore struct { @@ -357,6 +358,10 @@ func (b *Blockstore) IsBackwardsCompatible() bool { return false } +func (b *Blockstore) ID() string { + return SingleBlockstoreID +} + func (b *Blockstore) SigningKey() SecureString { return b.Signing.SecretKey } diff --git a/pkg/graveler/ref/manager.go b/pkg/graveler/ref/manager.go index 35ec46a9d43..ab394aced06 100644 --- a/pkg/graveler/ref/manager.go +++ b/pkg/graveler/ref/manager.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "github.com/treeverse/lakefs/pkg/config" "strings" "time" @@ -40,6 +41,7 @@ type Manager struct { commitCache cache.Cache maxBatchDelay time.Duration branchOwnership *distributed.MostlyCorrectOwner + storageConfig config.StorageConfig } func branchFromProto(pb *graveler.BranchData) *graveler.Branch { @@ -107,7 +109,7 @@ type ManagerConfig struct { BranchApproximateOwnershipParams BranchApproximateOwnershipParams } -func NewRefManager(cfg ManagerConfig) *Manager { +func NewRefManager(cfg ManagerConfig, storageCfg config.StorageConfig) *Manager { var branchOwnership *distributed.MostlyCorrectOwner if cfg.BranchApproximateOwnershipParams.RefreshInterval > 0 { log := logging.ContextUnavailable().WithField("component", "RefManager approximate branch ownership") @@ -130,6 +132,7 @@ func NewRefManager(cfg ManagerConfig) *Manager { commitCache: newCache(cfg.CommitCacheConfig), maxBatchDelay: cfg.MaxBatchDelay, branchOwnership: branchOwnership, + storageConfig: storageCfg, } } @@ -142,6 +145,10 @@ func (m *Manager) getRepository(ctx context.Context, repositoryID graveler.Repos } return nil, err } + repo := graveler.RepoFromProto(&data) + if repo.StorageID == config.SingleBlockstoreID { + repo.StorageID = graveler.StorageID(m.storageConfig.GetStorageByID(config.SingleBlockstoreID).ID()) // Will return the real actual ID + } return graveler.RepoFromProto(&data), nil } From a7292db9853a91d9375ddb153ec0e27b589bf6fe Mon Sep 17 00:00:00 2001 From: Nir Ozery Date: Tue, 25 Feb 2025 12:15:26 -0500 Subject: [PATCH 2/6] CR Fixes --- pkg/catalog/catalog.go | 3 ++- pkg/graveler/ref/manager.go | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/catalog/catalog.go b/pkg/catalog/catalog.go index 8db68c541d9..b3e21569a69 100644 --- a/pkg/catalog/catalog.go +++ b/pkg/catalog/catalog.go @@ -364,7 +364,8 @@ func New(ctx context.Context, cfg Config) (*Catalog, error) { CommitCacheConfig: ref.CacheConfig(baseCfg.Graveler.CommitCache), MaxBatchDelay: baseCfg.Graveler.MaxBatchDelay, BranchApproximateOwnershipParams: makeBranchApproximateOwnershipParams(baseCfg.Graveler.BranchOwnership), - }, cfg.Config.StorageConfig(), + }, + cfg.Config.StorageConfig(), ) gcManager := retention.NewGarbageCollectionManager(tierFSParams.Adapter, refManager, baseCfg.Committed.BlockStoragePrefix) settingManager := settings.NewManager(refManager, cfg.KVStore) diff --git a/pkg/graveler/ref/manager.go b/pkg/graveler/ref/manager.go index ab394aced06..68f6b0a4dbb 100644 --- a/pkg/graveler/ref/manager.go +++ b/pkg/graveler/ref/manager.go @@ -4,13 +4,13 @@ import ( "context" "errors" "fmt" - "github.com/treeverse/lakefs/pkg/config" "strings" "time" "github.com/hashicorp/go-multierror" "github.com/treeverse/lakefs/pkg/batch" "github.com/treeverse/lakefs/pkg/cache" + "github.com/treeverse/lakefs/pkg/config" "github.com/treeverse/lakefs/pkg/distributed" "github.com/treeverse/lakefs/pkg/graveler" "github.com/treeverse/lakefs/pkg/httputil" @@ -147,7 +147,11 @@ func (m *Manager) getRepository(ctx context.Context, repositoryID graveler.Repos } repo := graveler.RepoFromProto(&data) if repo.StorageID == config.SingleBlockstoreID { - repo.StorageID = graveler.StorageID(m.storageConfig.GetStorageByID(config.SingleBlockstoreID).ID()) // Will return the real actual ID + storage := m.storageConfig.GetStorageByID(config.SingleBlockstoreID) + if storage != nil { + repo.StorageID = graveler.StorageID(storage.ID()) // Will return the real actual ID + } + } return graveler.RepoFromProto(&data), nil } From a09231edaa97f6fcf0972707fcc922ae4f1312d6 Mon Sep 17 00:00:00 2001 From: Nir Ozery Date: Tue, 25 Feb 2025 12:34:58 -0500 Subject: [PATCH 3/6] Fix lint --- pkg/graveler/ref/manager.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/graveler/ref/manager.go b/pkg/graveler/ref/manager.go index 68f6b0a4dbb..3f45ee5a577 100644 --- a/pkg/graveler/ref/manager.go +++ b/pkg/graveler/ref/manager.go @@ -147,12 +147,11 @@ func (m *Manager) getRepository(ctx context.Context, repositoryID graveler.Repos } repo := graveler.RepoFromProto(&data) if repo.StorageID == config.SingleBlockstoreID { - storage := m.storageConfig.GetStorageByID(config.SingleBlockstoreID) - if storage != nil { + if storage := m.storageConfig.GetStorageByID(config.SingleBlockstoreID); storage != nil { repo.StorageID = graveler.StorageID(storage.ID()) // Will return the real actual ID } - } + return graveler.RepoFromProto(&data), nil } From b357485f3193a98d05d58cc0ca93a464bc0da904 Mon Sep 17 00:00:00 2001 From: Nir Ozery Date: Tue, 25 Feb 2025 13:14:05 -0500 Subject: [PATCH 4/6] Second attempt --- pkg/api/controller.go | 14 ++++++++++++-- pkg/catalog/catalog.go | 5 +---- pkg/graveler/ref/manager.go | 12 +----------- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/pkg/api/controller.go b/pkg/api/controller.go index b6bd62c885c..f258489f477 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -1969,7 +1969,7 @@ func (c *Controller) ListRepositories(w http.ResponseWriter, r *http.Request, pa creationDate := repo.CreationDate.Unix() r := apigen.Repository{ Id: repo.Name, - StorageId: swag.String(repo.StorageID), + StorageId: swag.String(c.getRepoStorageID(repo.StorageID)), StorageNamespace: repo.StorageNamespace, CreationDate: creationDate, DefaultBranch: repo.DefaultBranch, @@ -2026,6 +2026,7 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo } // Validate storage ID exists + storageID = c.getRepoStorageID(storageID) if !slices.Contains(c.Config.StorageConfig().GetStorageIDs(), storageID) { c.handleAPIError(ctx, w, r, graveler.ErrInvalidStorageID) return @@ -2226,6 +2227,15 @@ func (c *Controller) DeleteRepository(w http.ResponseWriter, r *http.Request, re writeResponse(w, r, http.StatusNoContent, nil) } +func (c *Controller) getRepoStorageID(storageID string) string { + if storageID == config.SingleBlockstoreID { + if storage := c.Config.StorageConfig().GetStorageByID(config.SingleBlockstoreID); storage != nil { + return storage.ID() // Will return the real actual ID + } + } + return storageID +} + func (c *Controller) GetRepository(w http.ResponseWriter, r *http.Request, repository string) { if !c.authorize(w, r, permissions.Node{ Permission: permissions.Permission{ @@ -2244,7 +2254,7 @@ func (c *Controller) GetRepository(w http.ResponseWriter, r *http.Request, repos CreationDate: repo.CreationDate.Unix(), DefaultBranch: repo.DefaultBranch, Id: repo.Name, - StorageId: swag.String(repo.StorageID), + StorageId: swag.String(c.getRepoStorageID(repo.StorageID)), StorageNamespace: repo.StorageNamespace, ReadOnly: swag.Bool(repo.ReadOnly), } diff --git a/pkg/catalog/catalog.go b/pkg/catalog/catalog.go index b3e21569a69..bb2fab0e0fa 100644 --- a/pkg/catalog/catalog.go +++ b/pkg/catalog/catalog.go @@ -353,7 +353,6 @@ func New(ctx context.Context, cfg Config) (*Catalog, error) { storeLimiter := kv.NewStoreLimiter(cfg.KVStore, limiter) addressProvider := ident.NewHexAddressProvider() - refManager := ref.NewRefManager( ref.ManagerConfig{ Executor: executor, @@ -364,9 +363,7 @@ func New(ctx context.Context, cfg Config) (*Catalog, error) { CommitCacheConfig: ref.CacheConfig(baseCfg.Graveler.CommitCache), MaxBatchDelay: baseCfg.Graveler.MaxBatchDelay, BranchApproximateOwnershipParams: makeBranchApproximateOwnershipParams(baseCfg.Graveler.BranchOwnership), - }, - cfg.Config.StorageConfig(), - ) + }) gcManager := retention.NewGarbageCollectionManager(tierFSParams.Adapter, refManager, baseCfg.Committed.BlockStoragePrefix) settingManager := settings.NewManager(refManager, cfg.KVStore) if cfg.SettingsManagerOption != nil { diff --git a/pkg/graveler/ref/manager.go b/pkg/graveler/ref/manager.go index 3f45ee5a577..35ec46a9d43 100644 --- a/pkg/graveler/ref/manager.go +++ b/pkg/graveler/ref/manager.go @@ -10,7 +10,6 @@ import ( "github.com/hashicorp/go-multierror" "github.com/treeverse/lakefs/pkg/batch" "github.com/treeverse/lakefs/pkg/cache" - "github.com/treeverse/lakefs/pkg/config" "github.com/treeverse/lakefs/pkg/distributed" "github.com/treeverse/lakefs/pkg/graveler" "github.com/treeverse/lakefs/pkg/httputil" @@ -41,7 +40,6 @@ type Manager struct { commitCache cache.Cache maxBatchDelay time.Duration branchOwnership *distributed.MostlyCorrectOwner - storageConfig config.StorageConfig } func branchFromProto(pb *graveler.BranchData) *graveler.Branch { @@ -109,7 +107,7 @@ type ManagerConfig struct { BranchApproximateOwnershipParams BranchApproximateOwnershipParams } -func NewRefManager(cfg ManagerConfig, storageCfg config.StorageConfig) *Manager { +func NewRefManager(cfg ManagerConfig) *Manager { var branchOwnership *distributed.MostlyCorrectOwner if cfg.BranchApproximateOwnershipParams.RefreshInterval > 0 { log := logging.ContextUnavailable().WithField("component", "RefManager approximate branch ownership") @@ -132,7 +130,6 @@ func NewRefManager(cfg ManagerConfig, storageCfg config.StorageConfig) *Manager commitCache: newCache(cfg.CommitCacheConfig), maxBatchDelay: cfg.MaxBatchDelay, branchOwnership: branchOwnership, - storageConfig: storageCfg, } } @@ -145,13 +142,6 @@ func (m *Manager) getRepository(ctx context.Context, repositoryID graveler.Repos } return nil, err } - repo := graveler.RepoFromProto(&data) - if repo.StorageID == config.SingleBlockstoreID { - if storage := m.storageConfig.GetStorageByID(config.SingleBlockstoreID); storage != nil { - repo.StorageID = graveler.StorageID(storage.ID()) // Will return the real actual ID - } - } - return graveler.RepoFromProto(&data), nil } From 1fc7c7d77e801f9e69d32611e851db97273199a9 Mon Sep 17 00:00:00 2001 From: Nir Ozery Date: Tue, 25 Feb 2025 13:37:57 -0500 Subject: [PATCH 5/6] Add comment --- pkg/api/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/api/controller.go b/pkg/api/controller.go index f258489f477..e0691c5708b 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -2026,7 +2026,7 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo } // Validate storage ID exists - storageID = c.getRepoStorageID(storageID) + storageID = c.getRepoStorageID(storageID) // This returns the actual storageID for the repository if !slices.Contains(c.Config.StorageConfig().GetStorageIDs(), storageID) { c.handleAPIError(ctx, w, r, graveler.ErrInvalidStorageID) return From 94f28958582e3c133c58c2ac15ce08af1a8af04c Mon Sep 17 00:00:00 2001 From: Nir Ozery Date: Tue, 25 Feb 2025 13:42:22 -0500 Subject: [PATCH 6/6] rename method --- pkg/api/controller.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/api/controller.go b/pkg/api/controller.go index e0691c5708b..7019bd32c35 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -1969,7 +1969,7 @@ func (c *Controller) ListRepositories(w http.ResponseWriter, r *http.Request, pa creationDate := repo.CreationDate.Unix() r := apigen.Repository{ Id: repo.Name, - StorageId: swag.String(c.getRepoStorageID(repo.StorageID)), + StorageId: swag.String(c.getActualStorageID(repo.StorageID)), StorageNamespace: repo.StorageNamespace, CreationDate: creationDate, DefaultBranch: repo.DefaultBranch, @@ -2026,7 +2026,7 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo } // Validate storage ID exists - storageID = c.getRepoStorageID(storageID) // This returns the actual storageID for the repository + storageID = c.getActualStorageID(storageID) if !slices.Contains(c.Config.StorageConfig().GetStorageIDs(), storageID) { c.handleAPIError(ctx, w, r, graveler.ErrInvalidStorageID) return @@ -2227,7 +2227,8 @@ func (c *Controller) DeleteRepository(w http.ResponseWriter, r *http.Request, re writeResponse(w, r, http.StatusNoContent, nil) } -func (c *Controller) getRepoStorageID(storageID string) string { +// getActualStorageID - This returns the actual storageID of the storage +func (c *Controller) getActualStorageID(storageID string) string { if storageID == config.SingleBlockstoreID { if storage := c.Config.StorageConfig().GetStorageByID(config.SingleBlockstoreID); storage != nil { return storage.ID() // Will return the real actual ID @@ -2254,7 +2255,7 @@ func (c *Controller) GetRepository(w http.ResponseWriter, r *http.Request, repos CreationDate: repo.CreationDate.Unix(), DefaultBranch: repo.DefaultBranch, Id: repo.Name, - StorageId: swag.String(c.getRepoStorageID(repo.StorageID)), + StorageId: swag.String(c.getActualStorageID(repo.StorageID)), StorageNamespace: repo.StorageNamespace, ReadOnly: swag.Bool(repo.ReadOnly), }