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

Jump to bottom

fix: bring back concurrent export genesis in v2 #21554

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
+38 −11
Open

fix: bring back concurrent export genesis in v2 #21554

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ed5f01e
fix: bring back concurrent export genesis in v2
akhilkumarpilli Sep 5, 2024
958a386
update godoc
akhilkumarpilli Sep 5, 2024
f0027dc
improve code
akhilkumarpilli Sep 5, 2024
265d869
try other approach
akhilkumarpilli Sep 5, 2024
75a5751
get state at version
akhilkumarpilli Sep 5, 2024
6606640
revert change
akhilkumarpilli Sep 5, 2024
d4ca906
typo
akhilkumarpilli Sep 5, 2024
a91e7de
address comments
akhilkumarpilli Sep 10, 2024
2964763
fix lint
akhilkumarpilli Sep 10, 2024
1dce1ee
Merge branch 'main' into akhil/concurrent-export-genesis
akhilkumarpilli Sep 19, 2024
8e0c4c9
replace RunWithCtx with genesisCtx
akhilkumarpilli Sep 20, 2024
4d1a612
use stateFactory in genesis expoert
kocubinski Sep 20, 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
7 changes: 6 additions & 1 deletion runtime/v2/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,12 @@ func (a *AppBuilder[T]) Build(opts ...AppBuilderOption[T]) (*App[T], error) {
return nil
},
ExportGenesis: func(ctx context.Context, version uint64) ([]byte, error) {
genesisJson, err := a.app.moduleManager.ExportGenesisForModules(ctx)
state, err := a.app.db.StateAt(version)
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, fmt.Errorf("unable to get latest state: %w", err)
}

genesisJson, err := a.app.moduleManager.ExportGenesisForModules(ctx, a.app.stf, state)
if err != nil {
return nil, fmt.Errorf("failed to export genesis: %w", err)
}
Expand Down
39 changes: 31 additions & 8 deletions runtime/v2/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import (
appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/core/legacy"
"cosmossdk.io/core/registry"
"cosmossdk.io/core/store"
"cosmossdk.io/core/transaction"
"cosmossdk.io/log"
"cosmossdk.io/server/v2/appmanager"
"cosmossdk.io/server/v2/stf"
)

Expand Down Expand Up @@ -194,6 +196,8 @@ func (m *MM[T]) InitGenesisJSON(
// ExportGenesisForModules performs export genesis functionality for modules
func (m *MM[T]) ExportGenesisForModules(
ctx context.Context,
appStf appmanager.StateTransitionFunction[T],
state store.ReaderMap,
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid Leaking Internal Interfaces in Module Manager API

Passing appStf appmanager.StateTransitionFunction[T] and state store.ReaderMap directly into the ExportGenesisForModules function exposes internal interfaces and implementation details at the module manager level. This can lead to tight coupling and makes it harder to maintain and refactor the code in the future.

Consider abstracting these dependencies or providing a higher-level interface that encapsulates the state transition functionality without exposing the internal types.

modulesToExport ...string,
) (map[string]json.RawMessage, error) {
if len(modulesToExport) == 0 {
Expand All @@ -204,17 +208,19 @@ func (m *MM[T]) ExportGenesisForModules(
return nil, err
}

type genesisResult struct {
bz json.RawMessage
err error
}

type ModuleI interface {
ExportGenesis(ctx context.Context) (json.RawMessage, error)
}

genesisData := make(map[string]json.RawMessage)

// TODO: make async export genesis https://github.com/cosmos/cosmos-sdk/issues/21303
channels := make(map[string]chan genesisResult)
for _, moduleName := range modulesToExport {
mod := m.modules[moduleName]
var moduleI ModuleI

if module, hasGenesis := mod.(appmodulev2.HasGenesis); hasGenesis {
moduleI = module.(ModuleI)
} else if module, hasABCIGenesis := mod.(appmodulev2.HasABCIGenesis); hasABCIGenesis {
Expand All @@ -223,12 +229,29 @@ func (m *MM[T]) ExportGenesisForModules(
continue
}

res, err := moduleI.ExportGenesis(ctx)
if err != nil {
return nil, err
channels[moduleName] = make(chan genesisResult)
go func(moduleI ModuleI, ch chan genesisResult) {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
_, _ = appStf.RunWithCtx(ctx, state, func(ctx context.Context) error {
jm, err := moduleI.ExportGenesis(ctx)
if err != nil {
ch <- genesisResult{nil, err}
return err
}
ch <- genesisResult{jm, nil}
return nil
})
return
Fixed Show fixed Hide fixed
}(moduleI, channels[moduleName])
}

genesisData := make(map[string]json.RawMessage)
for moduleName := range channels {
res := <-channels[moduleName]
if res.err != nil {
return nil, fmt.Errorf("genesis export error in %s: %w", moduleName, res.err)
}

genesisData[moduleName] = res
genesisData[moduleName] = res.bz
Comment on lines +245 to +252
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a deterministic order when collecting genesis data.

Collecting the exported genesis data from the channels using a range loop over the channels map may result in non-deterministic ordering. This is because the order of iteration over a map is not guaranteed to be the same across different runs.

To ensure deterministic ordering, consider the following:

  • Create a sorted slice of module names based on the keys of the channels map.
  • Iterate over the sorted slice of module names to collect the exported genesis data from the channels in a deterministic order.

Here's an example of how you can modify the code to achieve deterministic ordering:

 genesisData := make(map[string]json.RawMessage)
-for moduleName := range channels {
+moduleNames := make([]string, 0, len(channels))
+for moduleName := range channels {
+    moduleNames = append(moduleNames, moduleName)
+}
+sort.Strings(moduleNames)
+for _, moduleName := range moduleNames {
     res := <-channels[moduleName]
     if res.err != nil {
         return nil, fmt.Errorf("genesis export error in %s: %w", moduleName, res.err)
     }
     genesisData[moduleName] = res.bz
 }
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
genesisData := make(map[string]json.RawMessage)
for moduleName := range channels {
res := <-channels[moduleName]
if res.err != nil {
return nil, fmt.Errorf("genesis export error in %s: %w", moduleName, res.err)
}
genesisData[moduleName] = res
genesisData[moduleName] = res.bz
genesisData := make(map[string]json.RawMessage)
moduleNames := make([]string, 0, len(channels))
for moduleName := range channels {
moduleNames = append(moduleNames, moduleName)
}
sort.Strings(moduleNames)
for _, moduleName := range moduleNames {
res := <-channels[moduleName]
if res.err != nil {
return nil, fmt.Errorf("genesis export error in %s: %w", moduleName, res.err)
}
genesisData[moduleName] = res.bz

}

return genesisData, nil
Expand Down
19 changes: 3 additions & 16 deletions server/v2/appmanager/appmanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,24 +89,11 @@ func (a AppManager[T]) InitGenesis(

// ExportGenesis exports the genesis state of the application.
func (a AppManager[T]) ExportGenesis(ctx context.Context, version uint64) ([]byte, error) {
zeroState, err := a.db.StateAt(version)
if err != nil {
return nil, fmt.Errorf("unable to get latest state: %w", err)
if a.exportGenesis == nil {
return nil, errors.New("export genesis function not set")
}

bz := make([]byte, 0)
_, err = a.stf.RunWithCtx(ctx, zeroState, func(ctx context.Context) error {
if a.exportGenesis == nil {
return errors.New("export genesis function not set")
}

bz, err = a.exportGenesis(ctx, version)
if err != nil {
return fmt.Errorf("failed to export genesis state: %w", err)
}

return nil
})
bz, err := a.exportGenesis(ctx, version)
if err != nil {
return nil, fmt.Errorf("failed to export genesis state: %w", err)
}
Expand Down
Loading