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.

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

engine/cleanupmgr: finalize migration to job queue for cleanup operations #4218

Merged
merged 48 commits into from
Dec 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
0c3dbd6
cleanup: implement context-aware sleep and cleanup manager functionality
mastercactapus Dec 9, 2024
9cc35dd
cleanup: format code for consistency in alert store initialization
mastercactapus Dec 9, 2024
e5c72d7
cleanup: remove unused ConfigSource field from SetupArgs struct
mastercactapus Dec 9, 2024
a208406
Merge branch 'master' into cleanup-mgr-alert-jobs
mastercactapus Dec 10, 2024
7ba52bb
cleanup: refactor alert cleanup logic and add shift cleanup functiona…
mastercactapus Dec 10, 2024
bb0b63e
cleanup: refactor alert cleanup logic to use whileWork for better con…
mastercactapus Dec 10, 2024
a4cc12c
Merge branch 'cleanup-mgr-alert-jobs' into cleanup-mgr-shifts
mastercactapus Dec 10, 2024
859194a
cleanup: add comment
mastercactapus Dec 10, 2024
66b17bf
Merge branch 'cleanup-mgr-alert-jobs' into cleanup-mgr-shifts
mastercactapus Dec 10, 2024
be2a5d1
cleanup: remove unused cleanup statements from DB and update logic
mastercactapus Dec 10, 2024
f3bf6dd
test: refactor alert auto-close and cleanup tests for improved reliab…
mastercactapus Dec 10, 2024
7f6a499
Merge branch 'cleanup-mgr-alert-jobs' into cleanup-mgr-shifts
mastercactapus Dec 10, 2024
145d1c7
Merge branch 'master' into cleanup-mgr-shifts
mastercactapus Dec 12, 2024
eeea937
fix: update ShiftArgs Kind to reflect cleanup-manager-shifts
mastercactapus Dec 16, 2024
7976e6c
Merge branch 'master' into cleanup-mgr-shifts
mastercactapus Dec 16, 2024
26523b7
feat: add periodic jobs for schedule data cleanup and update queries
mastercactapus Dec 16, 2024
ff5d164
Merge branch 'master' into cleanup-mgr-schedule-data
mastercactapus Dec 16, 2024
2a28887
feat: add workers for cleanup of shifts and schedule data
mastercactapus Dec 16, 2024
f8c555d
fix: update periodic job interval for schedule data cleanup to 24 hours
mastercactapus Dec 17, 2024
9ee4383
feat: add logging support to cleanup manager and engine initialization
mastercactapus Dec 17, 2024
8494e91
refactor: streamline schedule data cleanup by extracting user validat…
mastercactapus Dec 17, 2024
fb7a782
feat: add logging for schedule data updates in cleanup manager
mastercactapus Dec 17, 2024
fdad66d
refactor: remove unnecessary checks for empty shifts in user and shif…
mastercactapus Dec 17, 2024
2c9ffe6
refactor: enhance schedule data cleanup by improving user validation …
mastercactapus Dec 17, 2024
8592ff8
refactor: improve formatting of schedule data update call in cleanup …
mastercactapus Dec 17, 2024
86f7b84
docs: add comment to clarify cleanupData function purpose in schedule…
mastercactapus Dec 17, 2024
0899d6c
feat: add CleanupAlertLogs function to manage alert log entries for d…
mastercactapus Dec 17, 2024
df8cdcb
refactor: remove unused cleanupAlertLogs statement and related logic …
mastercactapus Dec 17, 2024
634bd2b
feat: implement timeout for CleanupAlertLogs worker to handle longer …
mastercactapus Dec 17, 2024
dff614d
refactor: rename cleanupDays to more descriptive staleThresholdDays i…
mastercactapus Dec 17, 2024
bd31e64
docs: enhance comment for CleanupMgrScheduleData to clarify last_clea…
mastercactapus Dec 17, 2024
ab61a03
Merge branch 'master' into cleanup-mgr-schedule-data
mastercactapus Dec 17, 2024
a056d48
Merge branch 'cleanup-mgr-schedule-data' into cleanup-mgr-alertlog-jo…
mastercactapus Dec 17, 2024
fd5ef14
engine/cleanupmgr: refactor schedule data cleanup logic and add job f…
mastercactapus Dec 17, 2024
28896c5
Merge branch 'cleanup-mgr-sched-data-individual-jobs' into cleanup-mg…
mastercactapus Dec 17, 2024
b85bbdb
engine/cleanupmgr: implement alert log cleanup job scheduling and ref…
mastercactapus Dec 17, 2024
aa505b2
engine/initriver: remove unused noopWorker type
mastercactapus Dec 18, 2024
4bee028
engine/cleanupmgr: rename LookForWorkArgs types for consistency
mastercactapus Dec 18, 2024
00155fb
Merge branch 'master' into cleanup-mgr-sched-data-individual-jobs
mastercactapus Dec 18, 2024
c4962e9
Merge branch 'cleanup-mgr-sched-data-individual-jobs' into cleanup-mg…
mastercactapus Dec 18, 2024
4158efd
Merge branch 'master' into cleanup-mgr-alertlog-job-queue
mastercactapus Dec 18, 2024
acc14da
feat(cleanupmanager): implement API key cleanup functionality
mastercactapus Dec 18, 2024
77a229c
feat(cleanupmanager): add periodic job for API key cleanup and remove…
mastercactapus Dec 18, 2024
d8e3203
Merge branch 'master' into cleanup-mgr-api-keys
mastercactapus Dec 23, 2024
256dd0f
fix merge issue
mastercactapus Dec 23, 2024
483ebc9
fix merge issue
mastercactapus Dec 23, 2024
a63d12b
regen
mastercactapus Dec 23, 2024
75e2825
fix arg type
mastercactapus Dec 23, 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
50 changes: 50 additions & 0 deletions engine/cleanupmanager/apikeys.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package cleanupmanager

import (
"context"
"database/sql"
"fmt"

"github.com/riverqueue/river"
"github.com/target/goalert/config"
"github.com/target/goalert/gadb"
)

type APIKeysArgs struct{}

func (APIKeysArgs) Kind() string { return "cleanup-manager-api-keys" }

// CleanupAPIKeys will revoke access to the API from unused tokens, including both user sessions and calendar subscriptions.
func (db *DB) CleanupAPIKeys(ctx context.Context, j *river.Job[APIKeysArgs]) error {
err := db.whileWork(ctx, func(ctx context.Context, tx *sql.Tx) (done bool, err error) {
// After 30 days, the token is no longer valid, so delete it.
//
// This is defined by how the keyring system works for session signing, and is not influenced by the APIKeyExpireDays config.
count, err := gadb.New(tx).CleanupMgrDeleteOldSessions(ctx, 30)
if err != nil {
return false, fmt.Errorf("delete old user sessions: %w", err)
}
return count < 100, nil
})
if err != nil {
return err
}

cfg := config.FromContext(ctx)
if cfg.Maintenance.APIKeyExpireDays <= 0 {
return nil
}

err = db.whileWork(ctx, func(ctx context.Context, tx *sql.Tx) (done bool, err error) {
count, err := gadb.New(tx).CleanupMgrDisableOldCalSub(ctx, int32(cfg.Maintenance.APIKeyExpireDays))
if err != nil {
return false, fmt.Errorf("disable unused calsub keys: %w", err)
}
return count < 100, nil
})
if err != nil {
return err
}

return nil
}
17 changes: 1 addition & 16 deletions engine/cleanupmanager/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,13 @@ import (

"github.com/target/goalert/alert"
"github.com/target/goalert/engine/processinglock"
"github.com/target/goalert/util"
)

// DB handles updating escalation policies.
type DB struct {
db *sql.DB
lock *processinglock.Lock

cleanupAPIKeys *sql.Stmt
setTimeout *sql.Stmt

cleanupSessions *sql.Stmt

alertStore *alert.Store

logger *slog.Logger
Expand All @@ -38,20 +32,11 @@ func NewDB(ctx context.Context, db *sql.DB, alertstore *alert.Store, log *slog.L
return nil, err
}

p := &util.Prepare{Ctx: ctx, DB: db}

return &DB{
db: db,
lock: lock,
logger: log,

// Abort any cleanup operation that takes longer than 3 seconds
// error will be logged.
setTimeout: p.P(`SET LOCAL statement_timeout = 3000`),
cleanupAPIKeys: p.P(`update user_calendar_subscriptions set disabled = true where id = any(select id from user_calendar_subscriptions where greatest(last_access, last_update) < (now() - $1::interval) order by id limit 100 for update skip locked)`),

cleanupSessions: p.P(`DELETE FROM auth_user_sessions WHERE id = any(select id from auth_user_sessions where last_access_at < (now() - '30 days'::interval) LIMIT 100 for update skip locked)`),

alertStore: alertstore,
}, p.Err
}, nil
}
34 changes: 34 additions & 0 deletions engine/cleanupmanager/queries.sql
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,37 @@ _delete AS (
scope OFFSET @batch_size - 1
LIMIT 1;

-- name: CleanupMgrDeleteOldSessions :execrows
-- CleanupMgrDeleteOldSessions will delete old sessions from the auth_user_sessions table that are older than the given number of days before now.
DELETE FROM auth_user_sessions
WHERE id = ANY (
SELECT
id
FROM
auth_user_sessions
WHERE
last_access_at <(now() - '1 day'::interval * sqlc.arg(max_session_age_days)::int)
LIMIT 100
FOR UPDATE
SKIP LOCKED);

-- name: CleanupMgrDisableOldCalSub :execrows
-- CleanupMgrDeleteOldCalSub will disable old calendar subscriptions from the user_calendar_subscriptions table that are unused for at least the given number of days.
UPDATE
user_calendar_subscriptions
SET
disabled = TRUE
WHERE
id = ANY (
SELECT
id
FROM
user_calendar_subscriptions
WHERE
greatest(last_access, last_update) <(now() - '1 day'::interval * sqlc.arg(inactivity_threshold_days)::int)
ORDER BY
id
LIMIT 100
FOR UPDATE
SKIP LOCKED);

15 changes: 15 additions & 0 deletions engine/cleanupmanager/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const QueueName = "cleanup-manager"
const (
PriorityAlertCleanup = 1
PrioritySchedHistory = 1
PriorityAPICleanup = 1
PriorityTempSchedLFW = 2
PriorityAlertLogsLFW = 2
PriorityTempSched = 3
Expand Down Expand Up @@ -57,6 +58,7 @@ func (db *DB) Setup(ctx context.Context, args processinglock.SetupArgs) error {
river.AddWorker(args.Workers, river.WorkFunc(db.LookForWorkScheduleData))
river.AddWorker(args.Workers, river.WorkFunc(db.CleanupAlertLogs))
river.AddWorker(args.Workers, river.WorkFunc(db.LookForWorkAlertLogs))
river.AddWorker(args.Workers, river.WorkFunc(db.CleanupAPIKeys))

err := args.River.Queues().Add(QueueName, river.QueueConfig{MaxWorkers: 5})
if err != nil {
Expand Down Expand Up @@ -117,5 +119,18 @@ func (db *DB) Setup(ctx context.Context, args processinglock.SetupArgs) error {
),
})

args.River.PeriodicJobs().AddMany([]*river.PeriodicJob{
river.NewPeriodicJob(
river.PeriodicInterval(24*time.Hour),
func() (river.JobArgs, *river.InsertOpts) {
return APIKeysArgs{}, &river.InsertOpts{
Queue: QueueName,
Priority: PriorityAPICleanup,
}
},
&river.PeriodicJobOpts{RunOnStart: true},
),
})

return nil
}
56 changes: 0 additions & 56 deletions engine/cleanupmanager/update.go

This file was deleted.

52 changes: 52 additions & 0 deletions gadb/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading