Scheduler: max one instance of a job at a time

[mason-sharp]

Currently the scheduler will spawn a new job even if the previous one of the same type has not finished. That can happen for small values of run_frequency.

Instead, skip starting the new job if the previous one is still running.

Also, use max_connections for repset-diff and schema-diff discovery to align with the table-diff pools, even though in practice they only open a connection or two and then close them.

ACE-182

[coderabbitai]

📝 Walkthrough

Walkthrough

Connection pooling parameters in diff commands are now derived from configuration when not explicitly set, and passed to connection options. Job scheduler was enhanced to enforce singleton mode with reschedule limiting for both cron and duration-based scheduled jobs.

Changes

Cohort / File(s)	Summary
Configuration-driven Connection Pooling internal/consistency/diff/repset_diff.go, internal/consistency/diff/schema_diff.go	MaxConnections now defaults from config TableDiff.MaxConnections when unset, and PoolSize is passed to connection options for improved connection management.
Job Scheduler Singleton Mode internal/jobs/scheduler.go	Cron and duration-based job creation now includes gocron.WithSingletonMode(gocron.LimitModeReschedule) to enforce singleton scheduling behavior.

Poem

🐰 Pools configured with care,
Connections shared everywhere,
Jobs run solo, no overlaps found,
Scheduling harmony, all around!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: preventing multiple concurrent instances of the same scheduled job through singleton mode.
Description check	✅ Passed	The pull request description clearly describes the main changes: preventing overlapping job executions and applying max_connections configuration to repset-diff and schema-diff discovery pools, with a reference to issue ACE-182.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/singleton-job

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 2 duplication

Metric	Results
Duplication	2

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

[coderabbitai]

🧹 Nitpick comments (1)

internal/consistency/diff/schema_diff.go (1)

228-228: Optional: centralize schema-diff connection options to avoid drift.

Both call sites are correct, but using a small helper (like repset) keeps this consistent if options expand later.

♻️ Suggested refactor

@@
 func NewSchemaDiffTask() *SchemaDiffCmd {
@@
 }
+
+func (c *SchemaDiffCmd) connOpts() auth.ConnectionOptions {
+	return auth.ConnectionOptions{PoolSize: c.MaxConnections}
+}
@@
-		pool, err := auth.GetClusterNodeConnection(c.Ctx, nodeWithDBInfo, auth.ConnectionOptions{PoolSize: c.MaxConnections})
+		pool, err := auth.GetClusterNodeConnection(c.Ctx, nodeWithDBInfo, c.connOpts())
@@
-		pool, err := auth.GetClusterNodeConnection(task.Ctx, nodeWithDBInfo, auth.ConnectionOptions{PoolSize: task.MaxConnections})
+		pool, err := auth.GetClusterNodeConnection(task.Ctx, nodeWithDBInfo, task.connOpts())

Also applies to: 308-308

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/consistency/diff/schema_diff.go` at line 228, Centralize the
connection options used when calling auth.GetClusterNodeConnection by extracting
the auth.ConnectionOptions{PoolSize: c.MaxConnections} literal into a single
helper (e.g., a small function or a shared variable like buildConnectionOptions
or repSetConnectionOptions) and replace both call sites in schema_diff.go (calls
to GetClusterNodeConnection) to use that helper; ensure the helper accepts
context/cluster/node if needed or simply returns the ConnectionOptions so future
option additions are made in one place.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@internal/consistency/diff/schema_diff.go`:
- Line 228: Centralize the connection options used when calling
auth.GetClusterNodeConnection by extracting the auth.ConnectionOptions{PoolSize:
c.MaxConnections} literal into a single helper (e.g., a small function or a
shared variable like buildConnectionOptions or repSetConnectionOptions) and
replace both call sites in schema_diff.go (calls to GetClusterNodeConnection) to
use that helper; ensure the helper accepts context/cluster/node if needed or
simply returns the ConnectionOptions so future option additions are made in one
place.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e7ca0a21-99e5-4d90-a426-366430771d9a

📥 Commits

Reviewing files that changed from the base of the PR and between 587ab40 and d993bd7.

📒 Files selected for processing (3)

• internal/consistency/diff/repset_diff.go
• internal/consistency/diff/schema_diff.go
• internal/jobs/scheduler.go

[ibrarahmad]

LGTM