Skip to content
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

Deprecate peapod, provide migration to fstree #3013

Merged
merged 3 commits into from
Nov 15, 2024

Conversation

End-rey
Copy link
Contributor

@End-rey End-rey commented Nov 13, 2024

Closes #2924.

Is there anything else I should do in this pr?

Copy link

codecov bot commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 27.38589% with 175 lines in your changes missing coverage. Please review.

Project coverage is 22.84%. Comparing base (31fa3b4) to head (4fb969a).
Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
cmd/peapod-to-fstree/main.go 27.73% 156 Missing and 16 partials ⚠️
cmd/neofs-node/validate.go 0.00% 1 Missing and 1 partial ⚠️
cmd/neofs-lens/internal/peapod/root.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3013      +/-   ##
==========================================
+ Coverage   22.82%   22.84%   +0.01%     
==========================================
  Files         790      791       +1     
  Lines       58362    58603     +241     
==========================================
+ Hits        13323    13389      +66     
- Misses      44157    44316     +159     
- Partials      882      898      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@End-rey End-rey force-pushed the 2924-migration-peapod-to-fstree branch from 694842f to b189b3f Compare November 13, 2024 14:45
cmd/peapod-to-fstree/main.go Outdated Show resolved Hide resolved
cmd/peapod-to-fstree/main.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@End-rey End-rey force-pushed the 2924-migration-peapod-to-fstree branch from b189b3f to 649777b Compare November 14, 2024 15:24
Copy link
Member

@carpawell carpawell left a comment

Choose a reason for hiding this comment

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

I am almost sure we also want to migrate some metabase indexes in this util, otherwise i do not know who should do it.
See

if prm.StorageID == nil {
for i := range b.storage {
res, err := b.storage[i].Storage.Get(prm)
if err == nil || !errors.As(err, new(apistatus.ObjectNotFound)) {
return res, err
}
}
return common.GetRes{}, logicerr.Wrap(apistatus.ObjectNotFound{})
}
if len(prm.StorageID) == 0 {
return b.storage[len(b.storage)-1].Storage.Get(prm)
}
return b.storage[0].Storage.Get(prm)
,
return common.PutRes{
StorageID: storageID,
}, err
}
and
return common.GetRes{Object: obj, RawData: data}, nil

CHANGELOG.md Show resolved Hide resolved
cmd/peapod-to-fstree/main.go Outdated Show resolved Hide resolved
cmd/peapod-to-fstree/main.go Outdated Show resolved Hide resolved
@carpawell
Copy link
Member

Linter feels bad.

@End-rey End-rey force-pushed the 2924-migration-peapod-to-fstree branch from 649777b to 34fc018 Compare November 14, 2024 18:33
After migration from Peapod to Fstree, the config may have 1 substorage, if not
already migrated, there are 2 substorages.

Refs #2924.

Signed-off-by: Andrey Butusov <[email protected]>
Also suppress linter in `neofs-lens`.

Refs #2924.

Signed-off-by: Andrey Butusov <[email protected]>
@End-rey End-rey force-pushed the 2924-migration-peapod-to-fstree branch from 34fc018 to 7a17640 Compare November 14, 2024 18:34
cmd/peapod-to-fstree/main.go Outdated Show resolved Hide resolved
type epochState struct{}

func (s epochState) CurrentEpoch() uint64 {
return 0
Copy link
Member

Choose a reason for hiding this comment

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

Not sure what can be affected by this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can I easily get the epoch state in another way? The metabase requires it to be set.

Copy link
Member

@roman-khimov roman-khimov Nov 15, 2024

Choose a reason for hiding this comment

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

Only via network interaction. Maybe it's not a problem in our case (updating storage ID doesn't require epoch IIRC). Or maybe we can get some latest known one from meta or local state.

Copy link
Contributor Author

@End-rey End-rey Nov 15, 2024

Choose a reason for hiding this comment

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

I can get LastResyncEpoch from metabase:

// ReadLastResyncEpoch reads from db last epoch when metabase was resynchronized.
// If id is missing, returns 0, nil.
func (db *DB) ReadLastResyncEpoch() (epoch uint64, err error) {

But it will be necessary to recreate it after receiving an epoch.

Copy link
Member

Choose a reason for hiding this comment

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

It's somewhat different in meaning and it's a new one, 0.43.0 DB doesn't have it.

Copy link
Member

Choose a reason for hiding this comment

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

why do you need a precise epoch? just pick an epoch that does not claim anything about object expiration and allows GET and PUT freely, that is 0. @roman-khimov, what bothers you?

How can I easily get the epoch state in another way? The metabase requires it to be set.

i did this and i did not know there would be so many utils that just open metabases and look at them. normal metabase always has to know the current epoch (via internal state or via explicit parameter to every operation)

@End-rey End-rey force-pushed the 2924-migration-peapod-to-fstree branch from 7a17640 to bfe2f8e Compare November 15, 2024 12:03
}

srcPath := *nodeCfgPath
ss := strings.Split(srcPath, ".")
Copy link
Member

@carpawell carpawell Nov 15, 2024

Choose a reason for hiding this comment

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

i would recommend smth like

extension := filepath.Ext(srcPath)
name := srcPath[0:len(srcPath)-len(extension)]

dots are not prohibited in file names

Copy link
Member

Choose a reason for hiding this comment

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

In general, you can just do + ".migrated" and that's it. srcPath can be anything, with many dots, without dots, whatever.

@End-rey End-rey force-pushed the 2924-migration-peapod-to-fstree branch from bfe2f8e to 39f1a79 Compare November 15, 2024 15:07
Add `cmd/peapod-to-fstree` application which accepts YAML
configuration file of the storage node, for each configured shard,
overtakes data from Peapod to FSTree that has already been configured and
updates metabase `StorageId` indexes.

The tool is going to be used for phased and safe rejection of the
Peapod and the transition to FSTree.

Closes #2924.

Signed-off-by: Andrey Butusov <[email protected]>
@End-rey End-rey force-pushed the 2924-migration-peapod-to-fstree branch from 39f1a79 to 4fb969a Compare November 15, 2024 15:15
@roman-khimov roman-khimov merged commit f1b6982 into master Nov 15, 2024
22 checks passed
@roman-khimov roman-khimov deleted the 2924-migration-peapod-to-fstree branch November 15, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate peapod, provide migration to fstree
3 participants