Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
2 changes: 1 addition & 1 deletion coderd/apidoc/docs.go

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

2 changes: 1 addition & 1 deletion coderd/apidoc/swagger.json

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

18 changes: 17 additions & 1 deletion coderd/database/dbauthz/dbauthz.go
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,8 @@ var (
rbac.ResourceApiKey.Type: {policy.ActionDelete},
rbac.ResourceAibridgeInterception.Type: {policy.ActionDelete},
rbac.ResourceWorkspaceBuildOrchestration.Type: {policy.ActionDelete},
// Chat auto-archive sets archived=true on inactive chats.
// Chat auto-archive sets archived=true on inactive chats; the
// search_tsv backfill also updates chat messages.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This being a part of dbpurge feels a bit off.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Danny has an RFC in the works but I didn't want to block this until that got finalized.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless we have a timeline for when/if it will be actually implemented. It might be worthwhile making a copy of dbpurge, say dblivemigrate and plopping the implementation there.

If not, then at least make it very explicit with comments and possibly file naming that it's in the wrong place and meant to be moved once XXX lands.

rbac.ResourceChat.Type: {policy.ActionRead, policy.ActionUpdate},
// Purge old boundary logs past the retention period.
rbac.ResourceBoundaryLog.Type: {policy.ActionDelete},
Expand Down Expand Up @@ -1752,6 +1753,13 @@ func (q *querier) AutoArchiveInactiveChats(ctx context.Context, arg database.Aut
return q.db.AutoArchiveInactiveChats(ctx, arg)
}

func (q *querier) BackfillChatMessagesSearchTsv(ctx context.Context, batchSize int32) (int64, error) {
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceChat); err != nil {
return 0, err
}
return q.db.BackfillChatMessagesSearchTsv(ctx, batchSize)
}

func (q *querier) BackoffChatDiffStatus(ctx context.Context, arg database.BackoffChatDiffStatusParams) error {
// This is a system-level operation used by the gitsync
// background worker to reschedule failed refreshes. Same
Expand Down Expand Up @@ -1829,6 +1837,14 @@ func (q *querier) CalculateAIBridgeInterceptionsTelemetrySummary(ctx context.Con
return q.db.CalculateAIBridgeInterceptionsTelemetrySummary(ctx, arg)
}

func (q *querier) ChatSearchQueryIsEmpty(ctx context.Context, search string) (bool, error) {
// Pure function, no rows. Gates on chat read to match the listing caller.
Comment thread
johnstcn marked this conversation as resolved.
Outdated
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceChat); err != nil {
return false, err
}
return q.db.ChatSearchQueryIsEmpty(ctx, search)
}

func (q *querier) ClaimPrebuiltWorkspace(ctx context.Context, arg database.ClaimPrebuiltWorkspaceParams) (database.ClaimPrebuiltWorkspaceRow, error) {
empty := database.ClaimPrebuiltWorkspaceRow{}

Expand Down
8 changes: 8 additions & 0 deletions coderd/database/dbauthz/dbauthz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,14 @@ func (s *MethodTestSuite) TestChats() {
dbm.EXPECT().DeleteOldChats(gomock.Any(), database.DeleteOldChatsParams{}).Return(int64(0), nil).AnyTimes()
check.Args(database.DeleteOldChatsParams{}).Asserts(rbac.ResourceSystem, policy.ActionDelete)
}))
s.Run("BackfillChatMessagesSearchTsv", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
dbm.EXPECT().BackfillChatMessagesSearchTsv(gomock.Any(), int32(100)).Return(int64(0), nil).AnyTimes()
check.Args(int32(100)).Asserts(rbac.ResourceChat, policy.ActionUpdate)
}))
s.Run("ChatSearchQueryIsEmpty", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
dbm.EXPECT().ChatSearchQueryIsEmpty(gomock.Any(), "!!!").Return(true, nil).AnyTimes()
check.Args("!!!").Asserts(rbac.ResourceChat, policy.ActionRead)
}))
s.Run("GetChatRetentionDays", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
dbm.EXPECT().GetChatRetentionDays(gomock.Any()).Return(int32(30), nil).AnyTimes()
check.Args().Asserts()
Expand Down
16 changes: 16 additions & 0 deletions coderd/database/dbmetrics/querymetrics.go

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

30 changes: 30 additions & 0 deletions coderd/database/dbmock/dbmock.go

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

75 changes: 61 additions & 14 deletions coderd/database/dbpurge/dbpurge.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ const (
// Chat debug run deletions can cascade into steps with large JSONB
// payloads, so they use the same conservative batch size.
chatDebugRunsBatchSize = 1000
// 10k rows take ~800ms; capping at 5 batches bounds per-tick
// transaction growth at a few seconds. Larger backlogs drain
// across ticks.
Comment thread
johnstcn marked this conversation as resolved.
Outdated
chatSearchBackfillBatchSize = 10000
chatSearchBackfillMaxBatches = 5
)

type Option func(*instance)
Expand All @@ -61,6 +66,14 @@ func WithClock(clk quartz.Clock) Option {
return func(i *instance) { i.clk = clk }
}

// WithChatSearchBackfillLimits overrides backfill batch size and cap. For tests.
func WithChatSearchBackfillLimits(batchSize int32, maxBatches int) Option {
return func(i *instance) {
i.chatSearchBackfillBatchSize = batchSize
i.chatSearchBackfillMaxBatches = maxBatches
}
}

// New creates a new periodically purging database instance.
// Callers must Close the returned instance.
func New(ctx context.Context, logger slog.Logger, db database.Store, vals *codersdk.DeploymentValues, reg prometheus.Registerer, opts ...Option) io.Closer {
Expand All @@ -87,14 +100,26 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, vals *coder
}, []string{"record_type"})
reg.MustRegister(recordsPurged)

// Separate counter: the backfill updates rows, not purges them.
Comment thread
johnstcn marked this conversation as resolved.
Outdated
chatSearchRowsBackfilled := prometheus.NewCounter(prometheus.CounterOpts{
Namespace: "coderd",
Subsystem: "dbpurge",
Name: "chat_search_rows_backfilled_total",
Help: "Total number of chat message rows whose search_tsv was backfilled.",
})
reg.MustRegister(chatSearchRowsBackfilled)

inst := &instance{
cancel: cancelFunc,
closed: closed,
logger: logger,
vals: vals,
clk: quartz.NewReal(),
iterationDuration: iterationDuration,
recordsPurged: recordsPurged,
cancel: cancelFunc,
closed: closed,
logger: logger,
vals: vals,
clk: quartz.NewReal(),
iterationDuration: iterationDuration,
recordsPurged: recordsPurged,
chatSearchRowsBackfilled: chatSearchRowsBackfilled,
chatSearchBackfillBatchSize: chatSearchBackfillBatchSize,
chatSearchBackfillMaxBatches: chatSearchBackfillMaxBatches,
}
for _, opt := range opts {
opt(inst)
Expand Down Expand Up @@ -310,6 +335,21 @@ func (i *instance) purgeTick(ctx context.Context, db database.Store, start time.
}
}

// Safe to run incrementally: queue membership is per-row, content is
// immutable after insert, and soft-deleted rows leave the index
// automatically.
Comment thread
johnstcn marked this conversation as resolved.
Outdated
var backfilledChatSearchRows int64
for range i.chatSearchBackfillMaxBatches {
n, err := tx.BackfillChatMessagesSearchTsv(ctx, i.chatSearchBackfillBatchSize)
if err != nil {
return xerrors.Errorf("backfill chat_messages.search_tsv: %w", err)
}
backfilledChatSearchRows += n
if n < int64(i.chatSearchBackfillBatchSize) {
break
}
}

i.logger.Debug(ctx, "purged old database entries",
slog.F("workspace_agent_logs", purgedWorkspaceAgentLogs),
slog.F("expired_api_keys", expiredAPIKeys),
Expand All @@ -322,6 +362,7 @@ func (i *instance) purgeTick(ctx context.Context, db database.Store, start time.
slog.F("chats", purgedChats),
slog.F("chat_files", purgedChatFiles),
slog.F("chat_debug_runs", purgedChatDebugRuns),
slog.F("chat_search_rows_backfilled", backfilledChatSearchRows),
slog.F("duration", i.clk.Since(start)),
)

Expand All @@ -338,6 +379,9 @@ func (i *instance) purgeTick(ctx context.Context, db database.Store, start time.
i.recordsPurged.WithLabelValues("chat_debug_runs").Add(float64(purgedChatDebugRuns))
i.recordsPurged.WithLabelValues("chat_files").Add(float64(purgedChatFiles))
}
if i.chatSearchRowsBackfilled != nil {
i.chatSearchRowsBackfilled.Add(float64(backfilledChatSearchRows))
}

// chatConfigErr is returned after the tx, so do not record this
// iteration as successful when only the deferred config read failed.
Expand All @@ -362,13 +406,16 @@ func (i *instance) purgeTick(ctx context.Context, db database.Store, start time.
}

type instance struct {
cancel context.CancelFunc
closed chan struct{}
logger slog.Logger
vals *codersdk.DeploymentValues
clk quartz.Clock
iterationDuration *prometheus.HistogramVec
recordsPurged *prometheus.CounterVec
cancel context.CancelFunc
closed chan struct{}
logger slog.Logger
vals *codersdk.DeploymentValues
clk quartz.Clock
iterationDuration *prometheus.HistogramVec
recordsPurged *prometheus.CounterVec
chatSearchRowsBackfilled prometheus.Counter
chatSearchBackfillBatchSize int32
chatSearchBackfillMaxBatches int
}

func (i *instance) Close() error {
Expand Down
Loading
Loading