From 38f94c1c2946f9835bb1ccc28bbb04439467df49 Mon Sep 17 00:00:00 2001 From: Cameron Date: Fri, 17 Feb 2023 14:21:54 -0500 Subject: [PATCH] satellite/reputation: if node is DQd or exited skip applying audit If a node is already disqualified or exited, don't bother going through the audit reputation code. If there's an additional reputation update to be made based on the automatic offline audit these nodes would get, the overlay DB rejects it anyway. Change-Id: I48ba151aa640b32f87e1285521be465d1f692a78 --- satellite/reputation/service.go | 32 ++++++++++++------- satellite/reputation/service_test.go | 48 ++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/satellite/reputation/service.go b/satellite/reputation/service.go index 0e28c2a96..a17e94492 100644 --- a/satellite/reputation/service.go +++ b/satellite/reputation/service.go @@ -105,6 +105,25 @@ func NewService(log *zap.Logger, overlay *overlay.Service, db DB, config Config) func (service *Service) ApplyAudit(ctx context.Context, nodeID storj.NodeID, reputation overlay.ReputationStatus, result AuditType) (err error) { defer mon.Task()(&ctx)(&err) + // There are some cases where the caller did not get updated reputation-status information. + // (Usually this means the node was offline, disqualified, or exited and we skipped creating an order limit for it.) + var nodeExited bool + if reputation.Email == "" { + dossier, err := service.overlay.Get(ctx, nodeID) + if err != nil { + return err + } + reputation = dossier.Reputation.Status + if dossier.ExitStatus.ExitFinishedAt != nil { + nodeExited = true + } + } + + // If the node is disqualified or exited, we do not need to apply the audit, so return nil. + if reputation.Disqualified != nil || nodeExited { + return nil + } + now := time.Now() statusUpdate, err := service.db.Update(ctx, UpdateRequest{ NodeID: nodeID, @@ -115,16 +134,6 @@ func (service *Service) ApplyAudit(ctx context.Context, nodeID storj.NodeID, rep return err } - // There are some cases where the caller did not get updated reputation-status information. - // (Usually this means the node was offline and we skipped creating an order limit for it.) - if reputation.Email == "" { - dossier, err := service.overlay.Get(ctx, nodeID) - if err != nil { - return err - } - reputation = dossier.Reputation.Status - } - // only update node if its health status has changed, or it's a newly vetted // node. // this prevents the need to require caller of ApplyAudit() to always know @@ -260,7 +269,8 @@ func (service *Service) Close() error { return nil } // hasReputationChanged determines if the current node reputation is different from the newly updated reputation. This // function will only consider the Disqualified, UnknownAudiSuspended and OfflineSuspended statuses for changes. func hasReputationChanged(updated Info, current overlay.ReputationStatus, now time.Time) (changed bool, repChanges []nodeevents.Type) { - if statusChanged(current.Disqualified, updated.Disqualified) { + // there is no unDQ, so only update if changed from nil to not nil + if current.Disqualified == nil && updated.Disqualified != nil { repChanges = append(repChanges, nodeevents.Disqualified) changed = true } diff --git a/satellite/reputation/service_test.go b/satellite/reputation/service_test.go index fdbd184b8..ed29d3b8b 100644 --- a/satellite/reputation/service_test.go +++ b/satellite/reputation/service_test.go @@ -5,6 +5,7 @@ package reputation_test import ( "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -190,3 +191,50 @@ func TestDisqualificationAuditFailure(t *testing.T) { assert.NotNil(t, nodeInfo.Disqualified) }) } + +func TestExitedAndDQNodesGetNoAudit(t *testing.T) { + testplanet.Run(t, testplanet.Config{ + SatelliteCount: 1, StorageNodeCount: 3, UplinkCount: 0, + Reconfigure: testplanet.Reconfigure{ + Satellite: func(log *zap.Logger, index int, config *satellite.Config) { + config.Reputation.InitialAlpha = 1 + config.Reputation.AuditLambda = 1 + config.Reputation.AuditWeight = 1 + config.Reputation.AuditDQ = 0.4 + }, + }, + }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { + satel := planet.Satellites[0] + okNode := planet.StorageNodes[0].ID() + dqNode := planet.StorageNodes[1].ID() + exitNode := planet.StorageNodes[2].ID() + + // Ok node gets audit + require.NoError(t, satel.Reputation.Service.ApplyAudit(ctx, okNode, overlay.ReputationStatus{}, reputation.AuditOffline)) + info, err := satel.Reputation.Service.Get(ctx, okNode) + require.NoError(t, err) + require.Equal(t, int64(1), info.TotalAuditCount) + + // DQ node + require.NoError(t, satel.Overlay.Service.DisqualifyNode(ctx, dqNode, overlay.DisqualificationReasonAuditFailure)) + require.NoError(t, satel.Reputation.Service.ApplyAudit(ctx, dqNode, overlay.ReputationStatus{}, reputation.AuditOffline)) + info, err = satel.Reputation.Service.Get(ctx, dqNode) + require.NoError(t, err) + require.Zero(t, info.TotalAuditCount) + + // Exit node + now := time.Now() + _, err = satel.Overlay.DB.UpdateExitStatus(ctx, &overlay.ExitStatusRequest{ + NodeID: exitNode, + ExitInitiatedAt: now, + ExitLoopCompletedAt: now, + ExitFinishedAt: now, + ExitSuccess: true, + }) + require.NoError(t, err) + require.NoError(t, satel.Reputation.Service.ApplyAudit(ctx, exitNode, overlay.ReputationStatus{}, reputation.AuditOffline)) + info, err = satel.Reputation.Service.Get(ctx, exitNode) + require.NoError(t, err) + require.Zero(t, info.TotalAuditCount) + }) +}