Skip to content

fix(database): reset scheduler_cluster postgres sequence after seeding default cluster#4857

Open
anxkhn wants to merge 1 commit into
dragonflyoss:mainfrom
anxkhn:patch-5
Open

fix(database): reset scheduler_cluster postgres sequence after seeding default cluster#4857
anxkhn wants to merge 1 commit into
dragonflyoss:mainfrom
anxkhn:patch-5

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 6, 2026

Copy link
Copy Markdown
When seeding the default clusters, `seed()` in `manager/database/database.go`
inserts the default `SchedulerCluster` with an explicit `ID = 1` and then calls
`setPostgresSequence` to advance the owning Postgres serial past that value.
That call passed the wrong table name: it used `"seed_peer_cluster"` even though
the row just inserted is a `SchedulerCluster`.

As a result:

- The `scheduler_cluster` sequence is never advanced and stays at `1` after
  seeding.
- The `seed_peer_cluster` sequence is `setval`'d twice (once here, once in the
  seed-peer-cluster block a few lines below), which is harmless.

This points the reset at `"scheduler_cluster"` so its sequence is advanced after
the explicit-ID insert. The seed-peer-cluster block already resets
`"seed_peer_cluster"` correctly and is left unchanged.

```diff
-		if err := setPostgresSequence(db, "seed_peer_cluster", schedulerCluster.ID); err != nil {
+		if err := setPostgresSequence(db, "scheduler_cluster", schedulerCluster.ID); err != nil {

Related Issue

This completes the fix for the regression originally reported in #4752. The fix
that was merged for that issue advanced the wrong sequence for the scheduler
cluster, so the scheduler-cluster case described in #4752 still reproduces.

(#4752 is closed; I can open a fresh tracking issue if the maintainers prefer a
new open issue linked to this PR.)

Motivation and Context

On a fresh Postgres manager (migrate: true), seeding inserts
scheduler_cluster with id = 1 but leaves that table's serial sequence at 1.
The next insert into scheduler_cluster (for example the first
POST /api/v1/scheduler-clusters) draws id = 1 again and fails with
duplicate key value violates unique constraint "scheduler_cluster_pkey"
(SQLSTATE 23505), exactly the symptom from #4752. setPostgresSequence is a
no-op on non-Postgres drivers, and MySQL/MariaDB/PolarDB advance
AUTO_INCREMENT past explicit inserts, so only Postgres is affected.

Screenshots (if appropriate)

N/A

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation Update (if none of the other choices apply)

Checklist

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

Note on the "tests" checkbox: left unchecked deliberately. See the test-gap
explanation below; `manager/database` has no test harness and this path only runs
against a live Postgres.

…g default cluster

The postgres sequence reset after seeding the default scheduler cluster
passed "seed_peer_cluster" as the table name instead of "scheduler_cluster",
so setval targeted the wrong sequence. As a result the scheduler_cluster
serial sequence was never advanced past the explicitly inserted id=1, and
the seed_peer_cluster sequence was reset twice.

On a fresh Postgres manager this leaves the scheduler_cluster sequence at 1
after seeding, so creating a second scheduler cluster draws id=1 again and
fails with a duplicate key violation (SQLSTATE 23505), the exact regression
that dragonflyoss#4752 reported.

Point the reset at "scheduler_cluster" so its sequence is advanced after
the explicit-ID seed insert. The seed peer cluster block already resets
"seed_peer_cluster" correctly.

Refs dragonflyoss#4752

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 28.12%. Comparing base (1a0f5ce) to head (8329c73).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
manager/database/database.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4857   +/-   ##
=======================================
  Coverage   28.12%   28.12%           
=======================================
  Files         232      232           
  Lines       23157    23157           
=======================================
  Hits         6512     6512           
  Misses      16197    16197           
  Partials      448      448           
Flag Coverage Δ
unittests 28.12% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
manager/database/database.go 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gaius-qi gaius-qi left a comment

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.

LGTM

@gaius-qi gaius-qi added the bug label Jul 7, 2026

@hyy0322 hyy0322 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants