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

config: use P2PHybridIncomingConnectionsLimit only for hybrid mode #6103

Merged
merged 2 commits into from
Aug 28, 2024
Merged

config: use P2PHybridIncomingConnectionsLimit only for hybrid mode #6103

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
e0b352c
config: use P2PIncomingConnectionsLimit only for hybrid mode
algorandskiy Aug 15, 2024
e53ceb9
rename hybrid options to P2PHybrid
algorandskiy Aug 26, 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
4 changes: 2 additions & 2 deletions cmd/algocfg/profileCommand.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ var (

// P2P config defaults
cfg.EnableP2PHybridMode = true
cfg.P2PNetAddress = ":4190"
cfg.P2PHybridNetAddress = ":4190"
cfg.EnableDHTProviders = true
return cfg
},
Expand All @@ -125,7 +125,7 @@ var (

// P2P config defaults
cfg.EnableP2PHybridMode = true
cfg.P2PNetAddress = ":4190"
cfg.P2PHybridNetAddress = ":4190"
cfg.EnableDHTProviders = true
return cfg
},
Expand Down
9 changes: 5 additions & 4 deletions cmd/algocfg/profileCommand_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
package main

import (
"github.com/algorand/go-algorand/config"
"testing"

"github.com/algorand/go-algorand/config"

"github.com/stretchr/testify/require"

"github.com/algorand/go-algorand/test/partitiontest"
Expand Down Expand Up @@ -80,7 +81,7 @@ func Test_getConfigForArg(t *testing.T) {
require.Equal(t, config.PlaceholderPublicAddress, cfg.PublicAddress)

require.True(t, cfg.EnableP2PHybridMode)
require.Equal(t, ":4190", cfg.P2PNetAddress)
require.Equal(t, ":4190", cfg.P2PHybridNetAddress)
require.True(t, cfg.EnableDHTProviders)
})

Expand All @@ -100,7 +101,7 @@ func Test_getConfigForArg(t *testing.T) {
require.Equal(t, config.PlaceholderPublicAddress, cfg.PublicAddress)

require.True(t, cfg.EnableP2PHybridMode)
require.Equal(t, ":4190", cfg.P2PNetAddress)
require.Equal(t, ":4190", cfg.P2PHybridNetAddress)
require.True(t, cfg.EnableDHTProviders)
})

Expand All @@ -121,7 +122,7 @@ func Test_getConfigForArg(t *testing.T) {
require.Equal(t, "", cfg.PublicAddress)

require.True(t, cfg.EnableP2PHybridMode)
require.Equal(t, "", cfg.P2PNetAddress)
require.Equal(t, "", cfg.P2PHybridNetAddress)
require.True(t, cfg.EnableDHTProviders)
})
}
2 changes: 1 addition & 1 deletion config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func enrichNetworkingConfig(source Local) (Local, error) {
}
// In hybrid mode we want to prevent connections from the same node over both P2P and WS.
// The only way it is supported at the moment is to use net identity challenge that is based on PublicAddress.
if (source.NetAddress != "" || source.P2PNetAddress != "") && source.EnableP2PHybridMode && source.PublicAddress == "" {
if (source.NetAddress != "" || source.P2PHybridNetAddress != "") && source.EnableP2PHybridMode && source.PublicAddress == "" {
return source, errors.New("PublicAddress must be specified when EnableP2PHybridMode is set")
}
source.PublicAddress = strings.ToLower(source.PublicAddress)
Expand Down
34 changes: 22 additions & 12 deletions config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func TestLocal_EnrichNetworkingConfig(t *testing.T) {
require.ErrorContains(t, err, "PublicAddress must be specified when EnableP2PHybridMode is set")

c1 = Local{
P2PNetAddress: "test1",
P2PHybridNetAddress: "test1",
EnableP2PHybridMode: true,
}
c2, err = enrichNetworkingConfig(c1)
Expand Down Expand Up @@ -617,23 +617,27 @@ func TestLocal_IsGossipServer(t *testing.T) {
require.False(t, cfg.IsGossipServer())
require.False(t, cfg.IsWsGossipServer())
require.False(t, cfg.IsP2PGossipServer())
require.False(t, cfg.IsHybridServer())

cfg.NetAddress = ":4160"
require.True(t, cfg.IsGossipServer())
require.True(t, cfg.IsWsGossipServer())
require.False(t, cfg.IsP2PGossipServer())
require.False(t, cfg.IsHybridServer())

cfg.EnableGossipService = false
// EnableGossipService does not matter
require.True(t, cfg.IsGossipServer())
require.True(t, cfg.IsWsGossipServer())
require.False(t, cfg.IsP2PGossipServer())
require.False(t, cfg.IsHybridServer())

cfg.EnableP2P = true
cfg.NetAddress = ":4160"
require.True(t, cfg.IsGossipServer())
require.False(t, cfg.IsWsGossipServer())
require.True(t, cfg.IsP2PGossipServer())
require.False(t, cfg.IsHybridServer())

cfg.EnableP2P = false

Expand All @@ -642,41 +646,47 @@ func TestLocal_IsGossipServer(t *testing.T) {
require.True(t, cfg.IsGossipServer())
require.True(t, cfg.IsWsGossipServer())
require.False(t, cfg.IsP2PGossipServer())
require.False(t, cfg.IsHybridServer())

cfg.EnableP2PHybridMode = true
cfg.NetAddress = ""
require.False(t, cfg.IsGossipServer())
require.False(t, cfg.IsWsGossipServer())
require.False(t, cfg.IsP2PGossipServer())
require.False(t, cfg.IsHybridServer())

cfg.EnableP2PHybridMode = true
cfg.P2PNetAddress = ":4190"
cfg.P2PHybridNetAddress = ":4190"
require.True(t, cfg.IsGossipServer())
require.False(t, cfg.IsWsGossipServer())
require.True(t, cfg.IsP2PGossipServer())
require.False(t, cfg.IsHybridServer())

cfg.EnableP2PHybridMode = true
cfg.NetAddress = ":4160"
cfg.P2PNetAddress = ":4190"
cfg.P2PHybridNetAddress = ":4190"
require.True(t, cfg.IsGossipServer())
require.True(t, cfg.IsWsGossipServer())
require.True(t, cfg.IsP2PGossipServer())
require.True(t, cfg.IsHybridServer())

cfg.EnableP2PHybridMode = true
cfg.EnableP2P = true
cfg.NetAddress = ":4160"
cfg.P2PNetAddress = ":4190"
cfg.P2PHybridNetAddress = ":4190"
require.True(t, cfg.IsGossipServer())
require.True(t, cfg.IsWsGossipServer())
require.True(t, cfg.IsP2PGossipServer())
require.True(t, cfg.IsHybridServer())

cfg.EnableP2PHybridMode = true
cfg.EnableP2P = true
cfg.NetAddress = ":4160"
cfg.P2PNetAddress = ""
cfg.P2PHybridNetAddress = ""
require.True(t, cfg.IsGossipServer())
require.True(t, cfg.IsWsGossipServer())
require.False(t, cfg.IsP2PGossipServer())
require.False(t, cfg.IsHybridServer())
}

func TestLocal_RecalculateConnectionLimits(t *testing.T) {
Expand Down Expand Up @@ -720,23 +730,23 @@ func TestLocal_RecalculateConnectionLimits(t *testing.T) {
t.Parallel()

c := Local{
NetAddress: ":4160",
RestConnectionsSoftLimit: test.restSoftIn,
RestConnectionsHardLimit: test.restHardIn,
IncomingConnectionsLimit: test.incomingIn,
P2PIncomingConnectionsLimit: test.p2pIncomingIn,
NetAddress: ":4160",
RestConnectionsSoftLimit: test.restSoftIn,
RestConnectionsHardLimit: test.restHardIn,
IncomingConnectionsLimit: test.incomingIn,
P2PHybridIncomingConnectionsLimit: test.p2pIncomingIn,
}
if test.p2pIncomingIn > 0 {
c.EnableP2PHybridMode = true
c.P2PNetAddress = ":4190"
c.P2PHybridNetAddress = ":4190"
}
requireFDs := test.reservedIn + test.restHardIn + uint64(test.incomingIn) + uint64(test.p2pIncomingIn)
res := c.AdjustConnectionLimits(requireFDs, test.maxFDs)
require.Equal(t, test.updated, res)
require.Equal(t, int(test.restSoftExp), int(c.RestConnectionsSoftLimit))
require.Equal(t, int(test.restHardExp), int(c.RestConnectionsHardLimit))
require.Equal(t, int(test.incomingExp), int(c.IncomingConnectionsLimit))
require.Equal(t, int(test.p2pIncomingExp), int(c.P2PIncomingConnectionsLimit))
require.Equal(t, int(test.p2pIncomingExp), int(c.P2PHybridIncomingConnectionsLimit))
})
}
}
Expand Down
31 changes: 19 additions & 12 deletions config/localTemplate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ type Local struct {
// Estimating 1.5MB per incoming connection, 1.5MB*2400 = 3.6GB
IncomingConnectionsLimit int `version[0]:"-1" version[1]:"10000" version[17]:"800" version[27]:"2400"`

P2PIncomingConnectionsLimit int `version[34]:"1200"`
// P2PHybridIncomingConnectionsLimit is used as IncomingConnectionsLimit for P2P connections in hybrid mode.
// For pure P2P nodes IncomingConnectionsLimit is used.
P2PHybridIncomingConnectionsLimit int `version[34]:"1200"`

// BroadcastConnectionsLimit specifies the number of connections that
// will receive broadcast (gossip) messages from this node. If the
Expand Down Expand Up @@ -607,8 +609,8 @@ type Local struct {
// Enabling this setting also requires PublicAddress to be set.
EnableP2PHybridMode bool `version[34]:"false"`

// P2PNetAddress sets the listen address used for P2P networking, if hybrid mode is set.
P2PNetAddress string `version[34]:""`
// P2PHybridNetAddress sets the listen address used for P2P networking, if hybrid mode is set.
P2PHybridNetAddress string `version[34]:""`

// EnableDHT will turn on the hash table for use with capabilities advertisement
EnableDHTProviders bool `version[34]:"false"`
Expand Down Expand Up @@ -742,16 +744,21 @@ func (cfg Local) IsGossipServer() bool {
return cfg.IsWsGossipServer() || cfg.IsP2PGossipServer()
}

// IsWsGossipServer returns true if a node configured to run a listening ws net
// IsWsGossipServer returns true if a node is configured to run a listening ws net
func (cfg Local) IsWsGossipServer() bool {
// 1. NetAddress is set and EnableP2P is not set
// 2. NetAddress is set and EnableP2PHybridMode is set then EnableP2P is overridden by EnableP2PHybridMode
return cfg.NetAddress != "" && (!cfg.EnableP2P || cfg.EnableP2PHybridMode)
}

// IsP2PGossipServer returns true if a node configured to run a listening p2p net
// IsP2PGossipServer returns true if a node is configured to run a listening p2p net
func (cfg Local) IsP2PGossipServer() bool {
return (cfg.EnableP2P && !cfg.EnableP2PHybridMode && cfg.NetAddress != "") || (cfg.EnableP2PHybridMode && cfg.P2PNetAddress != "")
return (cfg.EnableP2P && !cfg.EnableP2PHybridMode && cfg.NetAddress != "") || (cfg.EnableP2PHybridMode && cfg.P2PHybridNetAddress != "")
}

// IsHybridServer returns true if a node configured to run a listening both ws and p2p networks
func (cfg Local) IsHybridServer() bool {
return cfg.NetAddress != "" && cfg.P2PHybridNetAddress != "" && cfg.EnableP2PHybridMode
}

// ensureAbsGenesisDir will convert a path to absolute, and will attempt to make a genesis directory there
Expand Down Expand Up @@ -950,22 +957,22 @@ func (cfg *Local) AdjustConnectionLimits(requiredFDs, maxFDs uint64) bool {
restDelta := diff + reservedRESTConns - cfg.RestConnectionsHardLimit
cfg.RestConnectionsHardLimit = reservedRESTConns
splitRatio := 1
if cfg.IsWsGossipServer() && cfg.IsP2PGossipServer() {
if cfg.IsHybridServer() {
// split the rest of the delta between ws and p2p evenly
splitRatio = 2
}
if cfg.IsWsGossipServer() {
if cfg.IsWsGossipServer() || cfg.IsP2PGossipServer() {
if cfg.IncomingConnectionsLimit > int(restDelta) {
cfg.IncomingConnectionsLimit -= int(restDelta) / splitRatio
} else {
cfg.IncomingConnectionsLimit = 0
}
}
if cfg.IsP2PGossipServer() {
if cfg.P2PIncomingConnectionsLimit > int(restDelta) {
cfg.P2PIncomingConnectionsLimit -= int(restDelta) / splitRatio
if cfg.IsHybridServer() {
if cfg.P2PHybridIncomingConnectionsLimit > int(restDelta) {
cfg.P2PHybridIncomingConnectionsLimit -= int(restDelta) / splitRatio
} else {
cfg.P2PIncomingConnectionsLimit = 0
cfg.P2PHybridIncomingConnectionsLimit = 0
}
}
} else {
Expand Down
4 changes: 2 additions & 2 deletions config/local_defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ var defaultLocal = Local{
OptimizeAccountsDatabaseOnStartup: false,
OutgoingMessageFilterBucketCount: 3,
OutgoingMessageFilterBucketSize: 128,
P2PIncomingConnectionsLimit: 1200,
P2PNetAddress: "",
P2PHybridIncomingConnectionsLimit: 1200,
P2PHybridNetAddress: "",
P2PPersistPeerID: false,
P2PPrivateKeyLocation: "",
ParticipationKeysRefreshInterval: 60000000000,
Expand Down
20 changes: 10 additions & 10 deletions daemon/algod/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,16 @@
if ot.Overflowed {
return errors.New("Initialize() overflowed when adding up ReservedHealthServiceConnections to the existing RLIMIT_NOFILE value; decrease RestConnectionsHardLimit")
}
if cfg.IsWsGossipServer() {
if cfg.IsGossipServer() {

Check warning on line 156 in daemon/algod/server.go

View check run for this annotation

Codecov / codecov/patch

daemon/algod/server.go#L156 

Added line #L156 was not covered by tests
fdRequired = ot.Add(fdRequired, uint64(cfg.IncomingConnectionsLimit))
if ot.Overflowed {
return errors.New("Initialize() overflowed when adding up IncomingConnectionsLimit to the existing RLIMIT_NOFILE value; decrease IncomingConnectionsLimit")
}
}
if cfg.IsP2PGossipServer() {
fdRequired = ot.Add(fdRequired, uint64(cfg.P2PIncomingConnectionsLimit))
if cfg.IsHybridServer() {
fdRequired = ot.Add(fdRequired, uint64(cfg.P2PHybridIncomingConnectionsLimit))

Check warning on line 163 in daemon/algod/server.go

View check run for this annotation

Codecov / codecov/patch

daemon/algod/server.go#L162-L163 

Added lines #L162 - L163 were not covered by tests
if ot.Overflowed {
return errors.New("Initialize() overflowed when adding up P2PIncomingConnectionsLimit to the existing RLIMIT_NOFILE value; decrease P2PIncomingConnectionsLimit")
return errors.New("Initialize() overflowed when adding up P2PHybridIncomingConnectionsLimit to the existing RLIMIT_NOFILE value; decrease P2PHybridIncomingConnectionsLimit")

Check warning on line 165 in daemon/algod/server.go

View check run for this annotation

Codecov / codecov/patch

daemon/algod/server.go#L165 

Added line #L165 was not covered by tests
}
}
_, hard, fdErr := util.GetFdLimits()
Expand All @@ -176,17 +176,17 @@
// but try to keep cfg.ReservedFDs untouched by decreasing other limits
if cfg.AdjustConnectionLimits(fdRequired, hard) {
s.log.Warnf(
"Updated connection limits: RestConnectionsSoftLimit=%d, RestConnectionsHardLimit=%d, IncomingConnectionsLimit=%d, P2PIncomingConnectionsLimit=%d",
"Updated connection limits: RestConnectionsSoftLimit=%d, RestConnectionsHardLimit=%d, IncomingConnectionsLimit=%d, P2PHybridIncomingConnectionsLimit=%d",

Check warning on line 179 in daemon/algod/server.go

View check run for this annotation

Codecov / codecov/patch

daemon/algod/server.go#L179 

Added line #L179 was not covered by tests
cfg.RestConnectionsSoftLimit,
cfg.RestConnectionsHardLimit,
cfg.IncomingConnectionsLimit,
cfg.P2PIncomingConnectionsLimit,
cfg.P2PHybridIncomingConnectionsLimit,

Check warning on line 183 in daemon/algod/server.go

View check run for this annotation

Codecov / codecov/patch

daemon/algod/server.go#L183 

Added line #L183 was not covered by tests
)
if cfg.IsWsGossipServer() && cfg.IncomingConnectionsLimit == 0 {
return errors.New("Initialize() failed to adjust connection limits")
if cfg.IsHybridServer() && cfg.P2PHybridIncomingConnectionsLimit == 0 {
return errors.New("Initialize() failed to adjust p2p hybrid connection limits")

Check warning on line 186 in daemon/algod/server.go

View check run for this annotation

Codecov / codecov/patch

daemon/algod/server.go#L185-L186 

Added lines #L185 - L186 were not covered by tests
}
if cfg.IsP2PGossipServer() && cfg.P2PIncomingConnectionsLimit == 0 {
return errors.New("Initialize() failed to adjust p2p connection limits")
if cfg.IsGossipServer() && cfg.IncomingConnectionsLimit == 0 {
return errors.New("Initialize() failed to adjust connection limits")

Check warning on line 189 in daemon/algod/server.go

View check run for this annotation

Codecov / codecov/patch

daemon/algod/server.go#L188-L189 

Added lines #L188 - L189 were not covered by tests
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions installer/config.json.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@
"OptimizeAccountsDatabaseOnStartup": false,
"OutgoingMessageFilterBucketCount": 3,
"OutgoingMessageFilterBucketSize": 128,
"P2PIncomingConnectionsLimit": 1200,
"P2PNetAddress": "",
"P2PHybridIncomingConnectionsLimit": 1200,
"P2PHybridNetAddress": "",
"P2PPersistPeerID": false,
"P2PPrivateKeyLocation": "",
"ParticipationKeysRefreshInterval": 60000000000,
Expand Down
4 changes: 2 additions & 2 deletions netdeploy/remote/deployedNetwork.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,8 +1009,8 @@
portList = append(portList, strconv.Itoa(port))
}
}
if node.P2PNetAddress != "" {
port, err = extractPublicPort(node.P2PNetAddress)
if node.P2PHybridNetAddress != "" {
port, err = extractPublicPort(node.P2PHybridNetAddress)

Check warning on line 1013 in netdeploy/remote/deployedNetwork.go

View check run for this annotation

Codecov / codecov/patch

netdeploy/remote/deployedNetwork.go#L1012-L1013 

Added lines #L1012 - L1013 were not covered by tests
if err != nil {
return
}
Expand Down
38 changes: 19 additions & 19 deletions netdeploy/remote/nodeConfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,25 @@ package remote

// NodeConfig represents the configuration settings to apply to a single node running on a host
type NodeConfig struct {
Name string `json:",omitempty"`
Wallets []NodeWalletData
NetAddress string `json:",omitempty"`
APIEndpoint string `json:",omitempty"`
APIToken string `json:",omitempty"`
AdminAPIToken string `json:",omitempty"`
EnableTelemetry bool // Needs to also be configured host-wide (assign logging host name)
TelemetryURI string `json:",omitempty"` // Needs to be HostConfig
EnableMetrics bool // Needs to also be configured host-wide (register DNS entry)
MetricsURI string `json:",omitempty"`
EnableService bool
CronTabSchedule string `json:",omitempty"`
EnableBlockStats bool
DashboardEndpoint string `json:",omitempty"`
DeadlockOverride int `json:",omitempty"` // -1 = Disable deadlock detection, 0 = Use Default for build, 1 = Enable
ConfigJSONOverride string `json:",omitempty"` // Raw json to merge into config.json after other modifications are complete
P2PBootstrap bool // True if this node should be a p2p bootstrap node and registered in DNS
P2PNetAddress string `json:",omitempty"`
PublicAddress bool
Name string `json:",omitempty"`
Wallets []NodeWalletData
NetAddress string `json:",omitempty"`
APIEndpoint string `json:",omitempty"`
APIToken string `json:",omitempty"`
AdminAPIToken string `json:",omitempty"`
EnableTelemetry bool // Needs to also be configured host-wide (assign logging host name)
TelemetryURI string `json:",omitempty"` // Needs to be HostConfig
EnableMetrics bool // Needs to also be configured host-wide (register DNS entry)
MetricsURI string `json:",omitempty"`
EnableService bool
CronTabSchedule string `json:",omitempty"`
EnableBlockStats bool
DashboardEndpoint string `json:",omitempty"`
DeadlockOverride int `json:",omitempty"` // -1 = Disable deadlock detection, 0 = Use Default for build, 1 = Enable
ConfigJSONOverride string `json:",omitempty"` // Raw json to merge into config.json after other modifications are complete
P2PBootstrap bool // True if this node should be a p2p bootstrap node and registered in DNS
P2PHybridNetAddress string `json:",omitempty"`
PublicAddress bool

// NodeNameMatchRegex is tested against Name in generated configs and if matched the rest of the configs in this record are applied as a template
NodeNameMatchRegex string `json:",omitempty"`
Expand Down
Loading
Loading