Add storageID to tier_fs #8549
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,12 +25,12 @@ func NewCommittedManager(m MetaRangeManager, r RangeManager, p Params) graveler. | |
| } | ||
| } | ||
|
|
||
| func (c *committedManager) Exists(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, id graveler.MetaRangeID) (bool, error) { | ||
| return c.metaRangeManager.Exists(ctx, storageID, ns, id) | ||
| } | ||
|
|
||
| func (c *committedManager) Get(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, rangeID graveler.MetaRangeID, key graveler.Key) (*graveler.Value, error) { | ||
| it, err := c.metaRangeManager.NewMetaRangeIterator(ctx, storageID, ns, rangeID) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
@@ -53,16 +53,16 @@ func (c *committedManager) Get(ctx context.Context, ns graveler.StorageNamespace | |
| return rec.Value, nil | ||
| } | ||
|
|
||
| func (c *committedManager) List(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, rangeID graveler.MetaRangeID) (graveler.ValueIterator, error) { | ||
| it, err := c.metaRangeManager.NewMetaRangeIterator(ctx, storageID, ns, rangeID) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return NewValueIterator(it), nil | ||
| } | ||
|
|
||
| func (c *committedManager) WriteRange(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, it graveler.ValueIterator) (*graveler.RangeInfo, error) { | ||
|
There was a problem hiding this comment. Can you please rename (Here, and in every function that have There was a problem hiding this comment. I hear what you're saying, but I don't think we should try to go into graveler and try to make all the names conform to some coding convention- that's not the goal of this PR There was a problem hiding this comment. It's not the goal, but a side-effect. Renaming it to There was a problem hiding this comment. This is like tabs/spaces. You end up with a commit war over code-style nonsense that's a huge waste of everyone's time. ns is an acceptable variable name for Go: https://www.reddit.com/r/golang/comments/3aporh/why_so_many_gophers_use_single_letter_variables/ I prefer a little more descriptive names, but I'm not going to go through our whole codebase and start correcting other people's naming There was a problem hiding this comment. So at least rename There was a problem hiding this comment. I think you can both be right!
|
||
| writer, err := c.RangeManager.GetWriter(ctx, StorageID(storageID), Namespace(ns), nil) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed creating range writer: %w", err) | ||
| } | ||
|
|
@@ -110,8 +110,8 @@ func (c *committedManager) WriteRange(ctx context.Context, ns graveler.StorageNa | |
| }, nil | ||
| } | ||
|
|
||
| func (c *committedManager) WriteMetaRange(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, ranges []*graveler.RangeInfo) (*graveler.MetaRangeInfo, error) { | ||
| writer := c.metaRangeManager.NewWriter(ctx, storageID, ns, nil) | ||
| defer func() { | ||
| if err := writer.Abort(); err != nil { | ||
| logging.FromContext(ctx).WithError(err).Error("Aborting write to meta range") | ||
|
|
@@ -146,8 +146,8 @@ func (c *committedManager) WriteMetaRange(ctx context.Context, ns graveler.Stora | |
| }, nil | ||
| } | ||
|
|
||
| func (c *committedManager) WriteMetaRangeByIterator(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, it graveler.ValueIterator, metadata graveler.Metadata) (*graveler.MetaRangeID, error) { | ||
| writer := c.metaRangeManager.NewWriter(ctx, storageID, ns, metadata) | ||
| defer func() { | ||
| if err := writer.Abort(); err != nil { | ||
| logging.FromContext(ctx).WithError(err).Error("Aborting write to meta range") | ||
|
|
@@ -170,20 +170,20 @@ func (c *committedManager) WriteMetaRangeByIterator(ctx context.Context, ns grav | |
| return id, nil | ||
| } | ||
|
|
||
| func (c *committedManager) Diff(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, left, right graveler.MetaRangeID) (graveler.DiffIterator, error) { | ||
| leftIt, err := c.metaRangeManager.NewMetaRangeIterator(ctx, storageID, ns, left) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| rightIt, err := c.metaRangeManager.NewMetaRangeIterator(ctx, storageID, ns, right) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return NewDiffValueIterator(ctx, leftIt, rightIt), nil | ||
| } | ||
|
|
||
| func (c *committedManager) Import(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, destination, source graveler.MetaRangeID, prefixes []graveler.Prefix, _ ...graveler.SetOptionsFunc) (graveler.MetaRangeID, error) { | ||
| destIt, err := c.metaRangeManager.NewMetaRangeIterator(ctx, storageID, ns, destination) | ||
| if err != nil { | ||
| return "", fmt.Errorf("get destination iterator: %w", err) | ||
| } | ||
|
|
@@ -228,6 +228,7 @@ type mergeContext struct { | |
| srcIt Iterator | ||
| baseIt Iterator | ||
| strategy graveler.MergeStrategy | ||
| storageID graveler.StorageID | ||
| ns graveler.StorageNamespace | ||
|
There was a problem hiding this comment. Can this also be renamed from |
||
| destinationID graveler.MetaRangeID | ||
| sourceID graveler.MetaRangeID | ||
|
|
@@ -238,7 +239,7 @@ func (c *committedManager) merge(ctx context.Context, mctx mergeContext) (gravel | |
| var err error = nil | ||
| baseIt := mctx.baseIt | ||
| if baseIt == nil { | ||
| baseIt, err = c.metaRangeManager.NewMetaRangeIterator(ctx, mctx.storageID, mctx.ns, mctx.baseID) | ||
| if err != nil { | ||
| return "", fmt.Errorf("get base iterator: %w", err) | ||
| } | ||
|
|
@@ -247,7 +248,7 @@ func (c *committedManager) merge(ctx context.Context, mctx mergeContext) (gravel | |
|
|
||
| destIt := mctx.destIt | ||
| if destIt == nil { | ||
| destIt, err = c.metaRangeManager.NewMetaRangeIterator(ctx, mctx.storageID, mctx.ns, mctx.destinationID) | ||
| if err != nil { | ||
| return "", fmt.Errorf("get destination iterator: %w", err) | ||
| } | ||
|
|
@@ -256,14 +257,14 @@ func (c *committedManager) merge(ctx context.Context, mctx mergeContext) (gravel | |
|
|
||
| srcIt := mctx.srcIt | ||
| if srcIt == nil { | ||
| srcIt, err = c.metaRangeManager.NewMetaRangeIterator(ctx, mctx.storageID, mctx.ns, mctx.sourceID) | ||
| if err != nil { | ||
| return "", fmt.Errorf("get source iterator: %w", err) | ||
| } | ||
| defer srcIt.Close() | ||
| } | ||
|
|
||
| mwWriter := c.metaRangeManager.NewWriter(ctx, mctx.storageID, mctx.ns, nil) | ||
| defer func() { | ||
| err = mwWriter.Abort() | ||
| if err != nil { | ||
|
|
@@ -285,15 +286,15 @@ func (c *committedManager) merge(ctx context.Context, mctx mergeContext) (gravel | |
| return *newID, err | ||
| } | ||
|
|
||
| func (c *committedManager) Commit(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, baseMetaRangeID graveler.MetaRangeID, changes graveler.ValueIterator, allowEmpty bool, _ ...graveler.SetOptionsFunc) (graveler.MetaRangeID, graveler.DiffSummary, error) { | ||
| mwWriter := c.metaRangeManager.NewWriter(ctx, storageID, ns, nil) | ||
| defer func() { | ||
| err := mwWriter.Abort() | ||
| if err != nil { | ||
| logging.FromContext(ctx).WithError(err).Error("Abort failed after Commit") | ||
| } | ||
| }() | ||
| metaRangeIterator, err := c.metaRangeManager.NewMetaRangeIterator(ctx, storageID, ns, baseMetaRangeID) | ||
| summary := graveler.DiffSummary{ | ||
| Count: map[graveler.DiffType]int{}, | ||
| } | ||
|
|
@@ -315,12 +316,12 @@ func (c *committedManager) Commit(ctx context.Context, ns graveler.StorageNamesp | |
| return *newID, summary, err | ||
| } | ||
|
|
||
| func (c *committedManager) Compare(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, destination, source, base graveler.MetaRangeID) (graveler.DiffIterator, error) { | ||
| diffIt, err := c.Diff(ctx, storageID, ns, destination, source) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("diff: %w", err) | ||
| } | ||
| baseIt, err := c.metaRangeManager.NewMetaRangeIterator(ctx, storageID, ns, base) | ||
| if err != nil { | ||
| diffIt.Close() | ||
| return nil, fmt.Errorf("get base iterator: %w", err) | ||
|
|
@@ -329,20 +330,20 @@ func (c *committedManager) Compare(ctx context.Context, ns graveler.StorageNames | |
| } | ||
|
|
||
| func (c *committedManager) GetMetaRange(ctx context.Context, ns graveler.StorageNamespace, id graveler.MetaRangeID) (graveler.MetaRangeAddress, error) { | ||
| uri, err := c.metaRangeManager.GetMetaRangeURI(ctx, id) | ||
| return graveler.MetaRangeAddress(uri), err | ||
| } | ||
|
|
||
| func (c *committedManager) GetRange(ctx context.Context, ns graveler.StorageNamespace, id graveler.RangeID) (graveler.RangeAddress, error) { | ||
| uri, err := c.metaRangeManager.GetRangeURI(ctx, id) | ||
| return graveler.RangeAddress(uri), err | ||
| } | ||
|
|
||
| func (c *committedManager) GetRangeIDByKey(ctx context.Context, storageID graveler.StorageID, ns graveler.StorageNamespace, id graveler.MetaRangeID, key graveler.Key) (graveler.RangeID, error) { | ||
| if id == "" { | ||
| return "", graveler.ErrNotFound | ||
| } | ||
| r, err := c.metaRangeManager.GetRangeByKey(ctx, storageID, ns, id, key) | ||
| if err != nil { | ||
| return "", fmt.Errorf("get range for key: %w", err) | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we expect the correct storageID here and in other places?