Skip to content

Commit da1f1a0

Browse files
committed
CR feedback on config selecting VP table size
1 parent 875a70b commit da1f1a0

File tree

5 files changed

+86
-9
lines changed

5 files changed

+86
-9
lines changed

config/config.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,3 +394,41 @@ func ApplyShorterUpgradeRoundsForDevNetworks(id protocol.NetworkID) {
394394
}
395395
}
396396
}
397+
398+
// NormalizeVoteCompressionTableSize validates and normalizes the VoteCompressionDynamicTableSize config value.
399+
// Supported values are powers of 2 in the range [16, 1024].
400+
// Values >= 1024 clamp to 1024.
401+
// Values 1-15 are below the minimum and return 0 (disabled).
402+
// Values between supported powers of 2 round down to the nearest supported value.
403+
// Logs a message if the configured value is adjusted.
404+
// Returns the normalized size.
405+
func NormalizeVoteCompressionTableSize(configured uint, log logger) uint {
406+
if configured == 0 {
407+
return 0
408+
}
409+
if configured < 16 {
410+
log.Warnf("VoteCompressionDynamicTableSize configured as %d is invalid (minimum 16). Dynamic vote compression disabled.", configured)
411+
return 0
412+
}
413+
if configured >= 1024 {
414+
if configured != 1024 {
415+
log.Infof("VoteCompressionDynamicTableSize configured as %d, using nearest supported value: 1024", configured)
416+
}
417+
return 1024
418+
}
419+
420+
// Round down to nearest power of 2 within supported range [16, 512]
421+
supportedSizes := []uint{512, 256, 128, 64, 32, 16}
422+
for _, size := range supportedSizes {
423+
if configured >= size {
424+
if configured != size {
425+
log.Infof("VoteCompressionDynamicTableSize configured as %d, using nearest supported value: %d", configured, size)
426+
}
427+
return size
428+
}
429+
}
430+
431+
// Should never reach here given the checks above
432+
log.Warnf("VoteCompressionDynamicTableSize configured as %d is invalid. Dynamic vote compression disabled.", configured)
433+
return 0
434+
}

config/config_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,10 @@ func (l tLogger) Infof(fmts string, args ...interface{}) {
861861
l.t.Logf(fmts, args...)
862862
}
863863

864+
func (l tLogger) Warnf(fmts string, args ...interface{}) {
865+
l.t.Logf(fmts, args...)
866+
}
867+
864868
// TestEnsureAndResolveGenesisDirs confirms that paths provided in the config are resolved to absolute paths and are created if relevant
865869
func TestEnsureAndResolveGenesisDirs(t *testing.T) {
866870
partitiontest.PartitionTest(t)

config/localTemplate.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,7 @@ func (cfg *Local) ResolveLogPaths(rootDir string) (liveLog, archive string) {
875875

876876
type logger interface {
877877
Infof(format string, args ...interface{})
878+
Warnf(format string, args ...interface{})
878879
}
879880

880881
// EnsureAndResolveGenesisDirs will resolve the supplied config paths to absolute paths, and will create the genesis directories of each

network/p2pNetwork.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,13 @@ type P2PNetwork struct {
5656
log logging.Logger
5757
config config.Local
5858
genesisInfo GenesisInfo
59-
ctx context.Context
60-
ctxCancel context.CancelFunc
61-
peerStats map[peer.ID]*p2pPeerStats
62-
peerStatsMu deadlock.Mutex
59+
// voteCompressionDynamicTableSize is the validated/normalized table size for VP compression.
60+
// It is set during setup() by validating config.VoteCompressionDynamicTableSize.
61+
voteCompressionDynamicTableSize uint
62+
ctx context.Context
63+
ctxCancel context.CancelFunc
64+
peerStats map[peer.ID]*p2pPeerStats
65+
peerStatsMu deadlock.Mutex
6366

6467
wg sync.WaitGroup
6568

@@ -353,6 +356,9 @@ func NewP2PNetwork(log logging.Logger, cfg config.Local, datadir string, phonebo
353356
}
354357

355358
func (n *P2PNetwork) setup() error {
359+
// Validate and normalize vote compression table size
360+
n.voteCompressionDynamicTableSize = config.NormalizeVoteCompressionTableSize(n.config.VoteCompressionDynamicTableSize, n.log)
361+
356362
if n.broadcaster.slowWritingPeerMonitorInterval == 0 {
357363
n.broadcaster.slowWritingPeerMonitorInterval = slowWritingPeerMonitorInterval
358364
}
@@ -830,6 +836,16 @@ func (n *P2PNetwork) Config() config.Local {
830836
return n.config
831837
}
832838

839+
// VoteCompressionDynamicTableSize returns the validated/normalized vote compression table size.
840+
func (n *P2PNetwork) VoteCompressionDynamicTableSize() uint {
841+
return n.voteCompressionDynamicTableSize
842+
}
843+
844+
// EnableVoteCompression returns whether vote compression is enabled for this node.
845+
func (n *P2PNetwork) EnableVoteCompression() bool {
846+
return n.config.EnableVoteCompression
847+
}
848+
833849
// wsStreamHandler is a callback that the p2p package calls when a new peer connects and establishes a
834850
// stream for the websocket protocol.
835851
// TODO: remove after consensus v41 takes effect.

network/wsNetwork.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,10 @@ type WebsocketNetwork struct {
167167

168168
config config.Local
169169

170+
// voteCompressionDynamicTableSize is the validated/normalized table size for VP compression.
171+
// It is set during setup() by validating config.VoteCompressionDynamicTableSize.
172+
voteCompressionDynamicTableSize uint
173+
170174
log logging.Logger
171175

172176
wg sync.WaitGroup
@@ -557,6 +561,9 @@ func (wn *WebsocketNetwork) setup() error {
557561
}
558562
wn.dialer = limitcaller.MakeRateLimitingDialer(wn.phonebook, preferredResolver)
559563

564+
// Validate and normalize vote compression table size
565+
wn.voteCompressionDynamicTableSize = config.NormalizeVoteCompressionTableSize(wn.config.VoteCompressionDynamicTableSize, wn.log)
566+
560567
wn.upgrader.ReadBufferSize = 4096
561568
wn.upgrader.WriteBufferSize = 4096
562569
wn.upgrader.EnableCompression = false
@@ -842,7 +849,8 @@ type peerMetadataProvider interface {
842849
PublicAddress() string
843850
RandomID() string
844851
SupportedProtoVersions() []string
845-
Config() config.Local
852+
EnableVoteCompression() bool
853+
VoteCompressionDynamicTableSize() uint
846854
}
847855

848856
// TelemetryGUID returns the telemetry GUID of this node.
@@ -870,6 +878,16 @@ func (wn *WebsocketNetwork) Config() config.Local {
870878
return wn.config
871879
}
872880

881+
// VoteCompressionDynamicTableSize returns the validated/normalized vote compression table size.
882+
func (wn *WebsocketNetwork) VoteCompressionDynamicTableSize() uint {
883+
return wn.voteCompressionDynamicTableSize
884+
}
885+
886+
// EnableVoteCompression returns whether vote compression is enabled for this node.
887+
func (wn *WebsocketNetwork) EnableVoteCompression() bool {
888+
return wn.config.EnableVoteCompression
889+
}
890+
873891
func setHeaders(header http.Header, netProtoVer string, meta peerMetadataProvider) {
874892
header.Set(TelemetryIDHeader, meta.TelemetryGUID())
875893
header.Set(InstanceNameHeader, meta.InstanceName())
@@ -883,14 +901,14 @@ func setHeaders(header http.Header, netProtoVer string, meta peerMetadataProvide
883901

884902
// set the features header (comma-separated list)
885903
features := []string{peerFeatureProposalCompression}
886-
if meta.Config().EnableVoteCompression {
904+
if meta.EnableVoteCompression() {
887905
features = append(features, peerFeatureVoteVpackCompression)
888906

889907
// Announce our maximum supported dynamic table size
890908
// Both sides will independently calculate min(ourSize, theirSize)
891909
// Only advertise dynamic features if stateless compression is enabled
892910
// Supported values: 16, 32, 64, 128, 256, 512, 1024 (or higher, which advertises 1024)
893-
switch dtSize := uint32(meta.Config().VoteCompressionDynamicTableSize); {
911+
switch dtSize := uint32(meta.VoteCompressionDynamicTableSize()); {
894912
case dtSize >= 1024:
895913
features = append(features, peerFeatureVoteVpackDynamic1024)
896914
case dtSize >= 512:
@@ -1143,7 +1161,7 @@ func (wn *WebsocketNetwork) ServeHTTP(response http.ResponseWriter, request *htt
11431161
identityVerified: atomic.Uint32{},
11441162
features: decodePeerFeatures(matchingVersion, request.Header.Get(PeerFeaturesHeader)),
11451163
enableVoteCompression: wn.config.EnableVoteCompression,
1146-
voteCompressionDynamicTableSize: wn.config.VoteCompressionDynamicTableSize,
1164+
voteCompressionDynamicTableSize: wn.voteCompressionDynamicTableSize,
11471165
}
11481166
peer.TelemetryGUID = trackedRequest.otherTelemetryGUID
11491167
wn.log.Debugf("Server: client features '%s', decoded %x, our response '%s'", request.Header.Get(PeerFeaturesHeader), peer.features, responseHeader.Get(PeerFeaturesHeader))
@@ -2211,7 +2229,7 @@ func (wn *WebsocketNetwork) tryConnect(netAddr, gossipAddr string) {
22112229
identity: peerID,
22122230
features: decodePeerFeatures(matchingVersion, response.Header.Get(PeerFeaturesHeader)),
22132231
enableVoteCompression: wn.config.EnableVoteCompression,
2214-
voteCompressionDynamicTableSize: wn.config.VoteCompressionDynamicTableSize,
2232+
voteCompressionDynamicTableSize: wn.voteCompressionDynamicTableSize,
22152233
}
22162234
peer.TelemetryGUID, peer.InstanceName, _ = getCommonHeaders(response.Header)
22172235
wn.log.Debugf("Client: server features '%s', decoded %x", response.Header.Get(PeerFeaturesHeader), peer.features)

0 commit comments

Comments
 (0)