Add support EC parameters #1065

Merged
fyrchik merged 3 commits from achuprov/frostfs-node:adm_ec_parameters into master 2024-09-04 19:51:07 +00:00
10 changed files with 62 additions and 3 deletions

View file

@ -34,6 +34,8 @@ alphabet-wallets: /home/user/deploy/alphabet-wallets
network:
max_object_size: 67108864
epoch_duration: 240
max_ec_data_count: 12

let's use only_small_letters

let's use `only_small_letters`

fixed

fixed
max_ec_parity_count: 4
fee:
candidate: 0
container: 0

View file

@ -26,6 +26,8 @@ const (
ContractsURLFlagDesc = "URL to archive with compiled FrostFS contracts"
EpochDurationInitFlag = "network.epoch_duration"
MaxObjectSizeInitFlag = "network.max_object_size"
MaxECDataCountFlag = "network.max_ec_data_count"
acid-ant marked this conversation as resolved Outdated

MaxECDataCountinitFlag -> MaxECDataCountFlag

MaxECDataCountinitFlag -> MaxECDataCountFlag

fixed

fixed
MaxECParityCounFlag = "network.max_ec_parity_count"
acid-ant marked this conversation as resolved Outdated

Config -> Flag.

Config -> Flag.
RefillGasAmountFlag = "gas"
StorageWalletFlag = "storage-wallet"
ContainerFeeInitFlag = "network.fee.container"

View file

@ -21,6 +21,8 @@ type configTemplate struct {
CandidateFee int
ContainerFee int
ContainerAliasFee int
MaxECDataCount int
MaxECParityCount int
WithdrawFee int
Glagolitics []string
HomomorphicHashDisabled bool
@ -31,6 +33,8 @@ alphabet-wallets: {{ .AlphabetDir}}
network:
max_object_size: {{ .MaxObjectSize}}
epoch_duration: {{ .EpochDuration}}
max_ec_data_count: {{ .MaxECDataCount}}
max_ec_parity_count: {{ .MaxECParityCount}}
homomorphic_hash_disabled: {{ .HomomorphicHashDisabled}}
fee:
candidate: {{ .CandidateFee}}
@ -106,6 +110,8 @@ func generateConfigExample(appDir string, credSize int) (string, error) {
tmpl := configTemplate{
Endpoint: "https://neo.rpc.node:30333",
MaxObjectSize: 67108864, // 64 MiB
MaxECDataCount: 12, // Tested with 16-node networks, assuming 12 data + 4 parity nodes.

The comment woudn't hurt, it is a magic constant, otherwise.

The comment woudn't hurt, it is a magic constant, otherwise.

The choice is indeed arbitrary.
The reasons are:

  1. 16-node networks is something we test regularly, 12+4.
  2. 4 parity chunks is more than enough (most policies have less or equal than 3 replicas)
The choice is indeed arbitrary. The reasons are: 1. 16-node networks is something we test regularly, `12+4`. 2. 4 parity chunks is more than enough (most policies have less or equal than 3 replicas)

fixed

fixed
MaxECParityCount: 4, // Maximum 4 parity chunks, typically <= 3 for most policies.
EpochDuration: 240, // 1 hour with 15s per block
HomomorphicHashDisabled: false, // object homomorphic hash is enabled
CandidateFee: 100_0000_0000, // 100.0 GAS (Fixed8)

View file

@ -27,6 +27,8 @@ func TestGenerateConfigExample(t *testing.T) {
require.Equal(t, "https://neo.rpc.node:30333", v.GetString("rpc-endpoint"))
require.Equal(t, filepath.Join(appDir, "alphabet-wallets"), v.GetString("alphabet-wallets"))
require.Equal(t, 67108864, v.GetInt("network.max_object_size"))
require.Equal(t, 12, v.GetInt("network.max_ec_data_count"))
acid-ant marked this conversation as resolved Outdated

Let's use only_small_letters here too.

Let's use only_small_letters here too.
require.Equal(t, 4, v.GetInt("network.max_ec_parity_count"))
require.Equal(t, 240, v.GetInt("network.epoch_duration"))
require.Equal(t, 10000000000, v.GetInt("network.fee.candidate"))
require.Equal(t, 1000, v.GetInt("network.fee.container"))

View file

@ -60,7 +60,8 @@ func dumpNetworkConfig(cmd *cobra.Command, _ []string) error {
switch k {
case netmap.ContainerFeeConfig, netmap.ContainerAliasFeeConfig,
netmap.EpochDurationConfig, netmap.IrCandidateFeeConfig,
netmap.MaxObjectSizeConfig, netmap.WithdrawFeeConfig:
netmap.MaxObjectSizeConfig, netmap.WithdrawFeeConfig,
netmap.MaxECDataCountConfig, netmap.MaxECParityCountConfig:
nbuf := make([]byte, 8)
copy(nbuf[:], v)
n := binary.LittleEndian.Uint64(nbuf)
@ -103,14 +104,22 @@ func SetConfigCmd(cmd *cobra.Command, args []string) error {
}
forceFlag, _ := cmd.Flags().GetBool(forceConfigSet)
bw := io.NewBufBinWriter()
prm := make(map[string]any)
for _, arg := range args {
k, v, err := parseConfigPair(arg, forceFlag)

This is a lot of code for a simple parameter addition, what is the reason?

This is a lot of code for a simple parameter addition, what is the reason?
if err != nil {
return err
}
prm[k] = v
}
acid-ant marked this conversation as resolved Outdated

Why we need to modify parameters for EC here?

Why we need to modify parameters for EC here?

There's a rule that the total of MaxECDataCount and MaxECParityCount can't be more than 256. If a user only picks one of these, we need to get the other from the network.

There's a rule that the total of `MaxECDataCount` and `MaxECParityCount` can't be more than 256. If a user only picks one of these, we need to get the other from the network.

If it is requirement to avoid setting one value without another, maybe it is better to set it only in pair?
In this case, we can skip getting the previous values and avoid iteration over the whole map for editing two keys.

If it is requirement to avoid setting one value without another, maybe it is better to set it only in pair? In this case, we can skip getting the previous values and avoid iteration over the whole map for editing two keys.

This will simplify the code, but this behavior may not be obvious to users

This will simplify the code, but this behavior may not be obvious to users

I have no objections to setting both: but we better still have 2 separate parameters, they just need to be provided simultaneously.

I have no objections to setting both: but we better still have 2 separate parameters, they just need to be provided simultaneously.

What is the reason to set configs for EC one by one? Because we have ability to skip check for 256 by calling contract method directly.

What is the reason to set configs for EC one by one? Because we have ability to skip check for `256` by calling contract method directly.

Because this is how they are stored in the netmap contract.

Because this is how they are stored in the netmap contract.

fixed

fixed
if err := validateConfig(prm, forceFlag); err != nil {
return err
}
for k, v := range prm {
// In NeoFS this is done via Notary contract. Here, however, we can form the
// transaction locally. The first `nil` argument is required only for notary
// disabled environment which is not supported by that command.
@ -128,6 +137,33 @@ func SetConfigCmd(cmd *cobra.Command, args []string) error {
return wCtx.AwaitTx()
}
func validateConfig(args map[string]any, forceFlag bool) error {
var sumEC int64
_, okData := args[netmap.MaxECDataCountConfig]
_, okParity := args[netmap.MaxECParityCountConfig]
if okData != okParity {

This is okData == okParity (which is also quite readable IMO: their existence status must be the same).

This is `okData == okParity` (which is also quite readable IMO: their existence status must be the same).

fixed

fixed
return fmt.Errorf("both %s and %s must be present in the configuration",
netmap.MaxECDataCountConfig, netmap.MaxECParityCountConfig)
}
for k, v := range args {
value, ok := v.(int64)
if !ok || value < 0 {
return fmt.Errorf("%s must be >= 0, got %v", k, v)
}
if k == netmap.MaxECDataCountConfig || k == netmap.MaxECParityCountConfig {
sumEC += value
}
}
if sumEC > 256 && !forceFlag {
return fmt.Errorf("the sum of %s and %s must be <= 256, got %d",
netmap.MaxECDataCountConfig, netmap.MaxECParityCountConfig, sumEC)
}
return nil
}
func parseConfigPair(kvStr string, force bool) (key string, val any, err error) {
k, v, found := strings.Cut(kvStr, "=")
if !found {
@ -140,7 +176,8 @@ func parseConfigPair(kvStr string, force bool) (key string, val any, err error)
switch key {
case netmap.ContainerFeeConfig, netmap.ContainerAliasFeeConfig,
netmap.EpochDurationConfig, netmap.IrCandidateFeeConfig,
netmap.MaxObjectSizeConfig, netmap.WithdrawFeeConfig:
netmap.MaxObjectSizeConfig, netmap.WithdrawFeeConfig,
netmap.MaxECDataCountConfig, netmap.MaxECParityCountConfig:
val, err = strconv.ParseInt(valRaw, 10, 64)
if err != nil {
err = fmt.Errorf("could not parse %s's value '%s' as int: %w", key, valRaw, err)

View file

@ -35,6 +35,8 @@ func GetDefaultNetmapContractConfigMap() map[string]any {
m := make(map[string]any)
m[netmap.EpochDurationConfig] = viper.GetInt64(commonflags.EpochDurationInitFlag)
m[netmap.MaxObjectSizeConfig] = viper.GetInt64(commonflags.MaxObjectSizeInitFlag)
m[netmap.MaxECDataCountConfig] = viper.GetInt64(commonflags.MaxECDataCountFlag)
m[netmap.MaxECParityCountConfig] = viper.GetInt64(commonflags.MaxECParityCounFlag)
m[netmap.ContainerFeeConfig] = viper.GetInt64(commonflags.ContainerFeeInitFlag)
m[netmap.ContainerAliasFeeConfig] = viper.GetInt64(commonflags.ContainerAliasFeeInitFlag)
m[netmap.IrCandidateFeeConfig] = viper.GetInt64(commonflags.CandidateFeeInitFlag)

View file

@ -25,6 +25,8 @@ var Cmd = &cobra.Command{
_ = viper.BindPFlag(commonflags.EndpointFlag, cmd.Flags().Lookup(commonflags.EndpointFlag))
_ = viper.BindPFlag(commonflags.EpochDurationInitFlag, cmd.Flags().Lookup(epochDurationCLIFlag))
_ = viper.BindPFlag(commonflags.MaxObjectSizeInitFlag, cmd.Flags().Lookup(maxObjectSizeCLIFlag))
_ = viper.BindPFlag(commonflags.MaxECDataCountFlag, cmd.Flags().Lookup(commonflags.MaxECDataCountFlag))
_ = viper.BindPFlag(commonflags.MaxECParityCounFlag, cmd.Flags().Lookup(commonflags.MaxECParityCounFlag))
_ = viper.BindPFlag(commonflags.HomomorphicHashDisabledInitFlag, cmd.Flags().Lookup(homomorphicHashDisabledCLIFlag))
_ = viper.BindPFlag(commonflags.CandidateFeeInitFlag, cmd.Flags().Lookup(candidateFeeCLIFlag))
_ = viper.BindPFlag(commonflags.ContainerFeeInitFlag, cmd.Flags().Lookup(containerFeeCLIFlag))

View file

@ -43,6 +43,8 @@ var netInfoCmd = &cobra.Command{
cmd.Printf(format, "Epoch duration", netInfo.EpochDuration())
cmd.Printf(format, "Inner Ring candidate fee", netInfo.IRCandidateFee())
cmd.Printf(format, "Maximum object size", netInfo.MaxObjectSize())
cmd.Printf(format, "Maximum count of data shards", netInfo.MaxECDataCount())
cmd.Printf(format, "Maximum count of parity shards", netInfo.MaxECParityCount())
cmd.Printf(format, "Withdrawal fee", netInfo.WithdrawalFee())
acid-ant marked this conversation as resolved Outdated

Why have you removed cmd.Printf(format, "Maintenance mode allowed"...?

Why have you removed `cmd.Printf(format, "Maintenance mode allowed"...`?

fixed

fixed
cmd.Printf(format, "Homomorphic hashing disabled", netInfo.HomomorphicHashingDisabled())
cmd.Printf(format, "Maintenance mode allowed", netInfo.MaintenanceModeAllowed())

View file

@ -6,6 +6,8 @@ network:
basic_income_rate: 100000000
homomorphic_hash_disabled: false
maintenance_mode_allowed: true
max_ec_data_count: 12
max_ec_parity_count: 4
fee:
audit: 10000
candidate: 10000000000

View file

@ -11,6 +11,8 @@ import (
const (
MaxObjectSizeConfig = "MaxObjectSize"
MaxECParityCountConfig = "MaxECParityCount"
MaxECDataCountConfig = "MaxECDataCount"
EpochDurationConfig = "EpochDuration"
ContainerFeeConfig = "ContainerFee"
ContainerAliasFeeConfig = "ContainerAliasFee"