satellite/reputation: move Config fields into a Config struct

This has been a cause of some confusion, even though the fields are
labeled as being copies of config values.

Having them be under a field explicitly named "Config" makes this
clearer, plus, allows the values to be passed in simply as a copy
of the Config struct from the satellite, rather than copying the fields
individually (which can be error-prone, particularly as the AuditCount
field in UpdateRequest is apparently not the same thing as the
AuditCount field in reputation.Config).

Refs: https://github.com/storj/storj/issues/4601

Change-Id: I386953347b71068596618616934aa28e3245cdc1
This commit is contained in:
paul cannon 2022-05-06 20:34:56 -05:00 committed by Jeff Wendling
parent fcb39b4a0f
commit 5bc4c254d4
6 changed files with 82 additions and 79 deletions

View File

@ -39,7 +39,9 @@ func BenchmarkReputation(b *testing.B) {
_, err := reputationdb.Update(ctx, reputation.UpdateRequest{
NodeID: id,
AuditOutcome: reputation.AuditSuccess,
AuditHistory: testAuditHistoryConfig(),
Config: reputation.Config{
AuditHistory: testAuditHistoryConfig(),
},
}, time.Now())
require.NoError(b, err)
}
@ -51,7 +53,9 @@ func BenchmarkReputation(b *testing.B) {
_, err := reputationdb.Update(ctx, reputation.UpdateRequest{
NodeID: id,
AuditOutcome: reputation.AuditFailure,
AuditHistory: testAuditHistoryConfig(),
Config: reputation.Config{
AuditHistory: testAuditHistoryConfig(),
},
}, time.Now())
require.NoError(b, err)
}
@ -63,7 +67,9 @@ func BenchmarkReputation(b *testing.B) {
_, err := reputationdb.Update(ctx, reputation.UpdateRequest{
NodeID: id,
AuditOutcome: reputation.AuditUnknown,
AuditHistory: testAuditHistoryConfig(),
Config: reputation.Config{
AuditHistory: testAuditHistoryConfig(),
},
}, time.Now())
require.NoError(b, err)
}
@ -75,7 +81,9 @@ func BenchmarkReputation(b *testing.B) {
_, err := reputationdb.Update(ctx, reputation.UpdateRequest{
NodeID: id,
AuditOutcome: reputation.AuditOffline,
AuditHistory: testAuditHistoryConfig(),
Config: reputation.Config{
AuditHistory: testAuditHistoryConfig(),
},
}, time.Now())
require.NoError(b, err)
}

View File

@ -38,17 +38,10 @@ type Config struct {
type UpdateRequest struct {
NodeID storj.NodeID
AuditOutcome AuditType
// n.b. these are set values from the satellite.
// They are part of the UpdateRequest struct in order to be
// more easily accessible in satellite/satellitedb/reputation.go.
AuditCount int64
AuditLambda float64
AuditWeight float64
AuditDQ float64
SuspensionGracePeriod time.Duration
SuspensionDQEnabled bool
AuditsRequiredForVetting int64
AuditHistory AuditHistoryConfig
// Config is a copy of the Config struct from the satellite.
// It is part of the UpdateRequest struct in order to be more easily
// accessible from satellitedb code.
Config
}
// AuditHistoryConfig is a configuration struct defining time periods and thresholds for penalizing nodes for being offline.

View File

@ -36,10 +36,12 @@ func TestUpdate(t *testing.T) {
// 1 audit -> unvetted
updateReq := reputation.UpdateRequest{
NodeID: node.ID(),
AuditOutcome: reputation.AuditOffline,
AuditsRequiredForVetting: planet.Satellites[0].Config.Reputation.AuditCount,
AuditHistory: testAuditHistoryConfig(),
NodeID: node.ID(),
AuditOutcome: reputation.AuditOffline,
Config: reputation.Config{
AuditCount: planet.Satellites[0].Config.Reputation.AuditCount,
AuditHistory: testAuditHistoryConfig(),
},
}
nodeStats, err := db.Update(ctx, updateReq, time.Now())
require.NoError(t, err)
@ -87,16 +89,17 @@ func TestDBDisqualificationAuditFailure(t *testing.T) {
now := time.Now()
updateReq := reputation.UpdateRequest{
NodeID: nodeID,
AuditOutcome: reputation.AuditFailure,
AuditCount: 0,
AuditLambda: 1,
AuditWeight: 1,
AuditDQ: 0.99,
SuspensionGracePeriod: 0,
SuspensionDQEnabled: false,
AuditsRequiredForVetting: 0,
AuditHistory: reputation.AuditHistoryConfig{},
NodeID: nodeID,
AuditOutcome: reputation.AuditFailure,
Config: reputation.Config{
AuditLambda: 1,
AuditWeight: 1,
AuditDQ: 0.99,
SuspensionGracePeriod: 0,
SuspensionDQEnabled: false,
AuditCount: 0,
AuditHistory: reputation.AuditHistoryConfig{},
},
}
status, err := reputationDB.Update(ctx, updateReq, now)
@ -114,16 +117,17 @@ func TestDBDisqualificationSuspension(t *testing.T) {
now := time.Now().Truncate(time.Second).UTC()
updateReq := reputation.UpdateRequest{
NodeID: nodeID,
AuditOutcome: reputation.AuditUnknown,
AuditCount: 0,
AuditLambda: 1,
AuditWeight: 1,
AuditDQ: 0.99,
SuspensionGracePeriod: 0,
SuspensionDQEnabled: true,
AuditsRequiredForVetting: 0,
AuditHistory: reputation.AuditHistoryConfig{},
NodeID: nodeID,
AuditOutcome: reputation.AuditUnknown,
Config: reputation.Config{
AuditLambda: 1,
AuditWeight: 1,
AuditDQ: 0.99,
SuspensionGracePeriod: 0,
SuspensionDQEnabled: true,
AuditCount: 0,
AuditHistory: reputation.AuditHistoryConfig{},
},
}
// suspend node due to failed unknown audit
@ -147,22 +151,23 @@ func TestDBDisqualificationNodeOffline(t *testing.T) {
now := time.Now().Truncate(time.Second).UTC()
updateReq := reputation.UpdateRequest{
NodeID: nodeID,
AuditOutcome: reputation.AuditOffline,
AuditCount: 0,
AuditLambda: 0,
AuditWeight: 0,
AuditDQ: 0,
SuspensionGracePeriod: 0,
SuspensionDQEnabled: false,
AuditsRequiredForVetting: 0,
AuditHistory: reputation.AuditHistoryConfig{
WindowSize: 0,
TrackingPeriod: 1 * time.Second,
GracePeriod: 0,
OfflineThreshold: 1,
OfflineDQEnabled: true,
OfflineSuspensionEnabled: true,
NodeID: nodeID,
AuditOutcome: reputation.AuditOffline,
Config: reputation.Config{
AuditLambda: 0,
AuditWeight: 0,
AuditDQ: 0,
SuspensionGracePeriod: 0,
SuspensionDQEnabled: false,
AuditCount: 0,
AuditHistory: reputation.AuditHistoryConfig{
WindowSize: 0,
TrackingPeriod: 1 * time.Second,
GracePeriod: 0,
OfflineThreshold: 1,
OfflineDQEnabled: true,
OfflineSuspensionEnabled: true,
},
},
}

View File

@ -72,14 +72,7 @@ func (service *Service) ApplyAudit(ctx context.Context, nodeID storj.NodeID, rep
statusUpdate, err := service.db.Update(ctx, UpdateRequest{
NodeID: nodeID,
AuditOutcome: result,
AuditLambda: service.config.AuditLambda,
AuditWeight: service.config.AuditWeight,
AuditDQ: service.config.AuditDQ,
SuspensionGracePeriod: service.config.SuspensionGracePeriod,
SuspensionDQEnabled: service.config.SuspensionDQEnabled,
AuditsRequiredForVetting: service.config.AuditCount,
AuditHistory: service.config.AuditHistory,
Config: service.config,
}, now)
if err != nil {
return err

View File

@ -302,7 +302,9 @@ func TestOfflineAuditSuspensionDisabled(t *testing.T) {
req := reputation.UpdateRequest{
NodeID: nodeID,
AuditOutcome: reputation.AuditOffline,
AuditHistory: config,
Config: reputation.Config{
AuditHistory: config,
},
}
// check that unsuspended node does not get suspended
@ -369,21 +371,23 @@ func TestOfflineSuspend(t *testing.T) {
updateReq := reputation.UpdateRequest{
NodeID: nodeID,
AuditOutcome: reputation.AuditOffline,
AuditHistory: reputation.AuditHistoryConfig{
WindowSize: time.Hour,
TrackingPeriod: 2 * time.Hour,
GracePeriod: time.Hour,
OfflineThreshold: 0.6,
OfflineDQEnabled: true,
OfflineSuspensionEnabled: true,
},
Config: reputation.Config{
AuditHistory: reputation.AuditHistoryConfig{
WindowSize: time.Hour,
TrackingPeriod: 2 * time.Hour,
GracePeriod: time.Hour,
OfflineThreshold: 0.6,
OfflineDQEnabled: true,
OfflineSuspensionEnabled: true,
},
AuditLambda: 0.95,
AuditWeight: 1,
AuditDQ: 0.6,
SuspensionGracePeriod: time.Hour,
SuspensionDQEnabled: true,
AuditsRequiredForVetting: 0,
AuditLambda: 0.95,
AuditWeight: 1,
AuditDQ: 0.6,
SuspensionGracePeriod: time.Hour,
SuspensionDQEnabled: true,
AuditCount: 0,
},
}
// give node an offline audit

View File

@ -467,7 +467,7 @@ func (reputations *reputations) populateUpdateNodeStats(dbNode *dbx.Reputation,
OnlineScore: float64Field{set: true, value: auditHistoryResponse.NewScore},
}
if vettedAt == nil && updatedTotalAuditCount >= updateReq.AuditsRequiredForVetting {
if vettedAt == nil && updatedTotalAuditCount >= updateReq.AuditCount {
updateFields.VettedAt = timeField{set: true, value: now}
}