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.

Sign up for GitHub

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

refactor(server/v2): late bound storeBuilder #22206

Merged
merged 2 commits into from
Oct 10, 2024
Merged

refactor(server/v2): late bound storeBuilder #22206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a5fd07
refactor(server/v2): late bound storeBuilder
kocubinski Oct 9, 2024
bcab586
rm func
kocubinski Oct 9, 2024
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
87 changes: 8 additions & 79 deletions server/v2/store/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,13 @@ package store

import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/spf13/cobra"
"github.com/spf13/viper"

"cosmossdk.io/log"
serverv2 "cosmossdk.io/server/v2"
storev2 "cosmossdk.io/store/v2"
"cosmossdk.io/store/v2/db"
"cosmossdk.io/store/v2/root"
)

Expand Down Expand Up @@ -45,7 +41,7 @@ Supported app-db-backend types include 'goleveldb', 'rocksdb', 'pebbledb'.`,

logger := log.NewLogger(cmd.OutOrStdout())

rootStore, keepRecent, err := createRootStore(cmd, vp, logger)
rootStore, opts, err := createRootStore(vp, logger)
if err != nil {
return fmt.Errorf("can not create root store %w", err)
Comment on lines +44 to 46
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct spelling of 'cannot' in error message

The error message should use 'cannot' instead of 'can not' as per standard English usage.

Apply this diff to correct the spelling:

 return fmt.Errorf("can not create root store %w", err)
+return fmt.Errorf("cannot create root store %w", err)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
rootStore, opts, err := createRootStore(vp, logger)
if err != nil {
return fmt.Errorf("can not create root store %w", err)
rootStore, opts, err := createRootStore(vp, logger)
if err != nil {
return fmt.Errorf("cannot create root store %w", err)

}
Expand All @@ -60,7 +56,7 @@ Supported app-db-backend types include 'goleveldb', 'rocksdb', 'pebbledb'.`,
return fmt.Errorf("the database has no valid heights to prune, the latest height: %v", latestHeight)
}

diff := latestHeight - keepRecent
diff := latestHeight - opts.SCPruningOption.KeepRecent
cmd.Printf("pruning heights up to %v\n", diff)
Comment on lines +59 to 60
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle potential negative 'diff' value

If opts.SCPruningOption.KeepRecent is greater than latestHeight, the calculated diff will be negative, which may not make sense in the context of pruning. Consider adding a check to ensure that diff is not negative before proceeding.

Consider adding the following check:

+if diff <= 0 {
+    cmd.Println("No heights to prune")
+    return nil
+}
 diff := latestHeight - opts.SCPruningOption.KeepRecent
 cmd.Printf("pruning heights up to %v\n", diff)

Committable suggestion was skipped due to low confidence.


err = rootStore.Prune(latestHeight)
Expand All @@ -79,81 +75,14 @@ Supported app-db-backend types include 'goleveldb', 'rocksdb', 'pebbledb'.`,
return cmd
}

func createRootStore(cmd *cobra.Command, v *viper.Viper, logger log.Logger) (storev2.RootStore, uint64, error) {
tempViper := v
rootDir := v.GetString(serverv2.FlagHome)
// handle FlagAppDBBackend
var dbType db.DBType
if cmd.Flags().Changed(FlagAppDBBackend) {
dbStr, err := cmd.Flags().GetString(FlagAppDBBackend)
if err != nil {
return nil, 0, err
}
dbType = db.DBType(dbStr)
} else {
dbType = db.DBType(v.GetString(FlagAppDBBackend))
}
scRawDb, err := db.NewDB(dbType, "application", filepath.Join(rootDir, "data"), nil)
func createRootStore(v *viper.Viper, logger log.Logger) (storev2.RootStore, root.Options, error) {
storeConfig, err := UnmarshalConfig(v.AllSettings())
if err != nil {
panic(err)
}

// handle KeepRecent & Interval flags
if cmd.Flags().Changed(FlagKeepRecent) {
keepRecent, err := cmd.Flags().GetUint64(FlagKeepRecent)
if err != nil {
return nil, 0, err
}

// viper has an issue that we could not override subitem then Unmarshal key
// so we can not do viper.Set() as comment below
// https://github.com/spf13/viper/issues/1106
// Do it by a hacky: overwrite app.toml file then read config again.

// v.Set("store.options.sc-pruning-option.keep-recent", keepRecent) // entry that read from app.toml
// v.Set("store.options.ss-pruning-option.keep-recent", keepRecent)

err = overrideKeepRecent(filepath.Join(rootDir, "config"), keepRecent)
if err != nil {
return nil, 0, err
}

tempViper, err = serverv2.ReadConfig(filepath.Join(rootDir, "config"))
if err != nil {
return nil, 0, err
}
return nil, root.Options{}, fmt.Errorf("failed to unmarshal config: %w", err)
}

storeOpts := root.DefaultStoreOptions()
if v != nil && v.Sub("store.options") != nil {
if err := v.Sub("store.options").Unmarshal(&storeOpts); err != nil {
return nil, 0, fmt.Errorf("failed to store options: %w", err)
}
}

store, err := root.CreateRootStore(&root.FactoryOptions{
Logger: logger,
RootDir: rootDir,
Options: storeOpts,
SCRawDB: scRawDb,
})

return store, tempViper.GetUint64("store.options.sc-pruning-option.keep-recent"), err
}

func overrideKeepRecent(configPath string, keepRecent uint64) error {
bz, err := os.ReadFile(filepath.Join(configPath, "app.toml"))
store, err := root.NewBuilder().Build(logger, storeConfig)
if err != nil {
return err
return nil, root.Options{}, fmt.Errorf("failed to create store backend: %w", err)
}
lines := strings.Split(string(bz), "\n")

for i, line := range lines {
if strings.Contains(line, "keep-recent") {
lines[i] = fmt.Sprintf("keep-recent = %d", keepRecent)
}
}
output := strings.Join(lines, "\n")

return os.WriteFile(filepath.Join(configPath, "app.toml"), []byte(output), 0o600)
return store, storeConfig.Options, nil
}
3 changes: 1 addition & 2 deletions server/v2/store/snapshot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ func (s *Server[T]) ExportSnapshotCmd() *cobra.Command {
}

logger := log.NewLogger(cmd.OutOrStdout())
// app := appCreator(logger, db, nil, viper)
rootStore, _, err := createRootStore(cmd, v, logger)
rootStore, _, err := createRootStore(v, logger)
if err != nil {
return err
}
Expand Down
Loading