[WIP] feat(jobsdb): non-blocking dataset compaction (enabled by default) - #6979
Closed
atzoum wants to merge 1 commit into
Closed
[WIP] feat(jobsdb): non-blocking dataset compaction (enabled by default)#6979atzoum wants to merge 1 commit into
atzoum wants to merge 1 commit into
Conversation
This was referenced May 19, 2026
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 19, 2026 18:21
87083b5 to
1a6fb57
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 19, 2026 18:21
d7b985b to
9442274
Compare
atzoum
marked this pull request as draft
May 19, 2026 18:21
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6979 +/- ##
==========================================
+ Coverage 80.07% 80.12% +0.04%
==========================================
Files 566 566
Lines 64286 64286
==========================================
+ Hits 51480 51511 +31
+ Misses 9726 9705 -21
+ Partials 3080 3070 -10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 20, 2026 14:09
1a6fb57 to
e521871
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 20, 2026 14:09
9442274 to
29d5f57
Compare
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 21, 2026 07:08
e521871 to
d3d4436
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 21, 2026 07:08
29d5f57 to
64bbddf
Compare
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 22, 2026 13:57
d3d4436 to
61c77d7
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 22, 2026 13:57
f1b845e to
780eb86
Compare
atzoum
added a commit
that referenced
this pull request
May 22, 2026
🔒 Scanned for secrets using gitleaks 8.30.1 # Description `jobsdb` currently retains the `dsList` read lock for the full duration of the store callback. As a result, long-running store transactions can block `dsList` writers (`addNewDS`, `migrateDSLoop`). Once a writer begins waiting on the write lock, new `dsList` readers may also become blocked. Therefore, we are now snapshoting the last dataset under the read lock and release the lock immediately, so the store transaction can take as long as it needs without holding dsList writers back. If this turns out to cause regressions in production, the new reloadable config `jobsDB.holdDSListLockDuringStore` (default `false`) reverts to the previous hold-the-lock-throughout behavior, so no redeployment is required. ## Uncovered latent bugs Releasing the read lock early exposed two latent bugs in existing tests: 1. **`WithStoreSafeTxFromTx` couldn't retry on a stale ds.** When the caller owned the outer `*Tx` and the store returned `errStaleDsList`, the savepoint inside `doStoreJobsInTx` was never rolled back. The retry inherited an aborted savepoint and the whole transaction failed. Fixed by rolling back to the savepoint before returning `ErrStaleDsList`. 2. **The JSON sanitize path was missing the same stale-ds handling.** Once the rollback was in place, it became obvious that the post-sanitize `store()` retry could itself hit the read-only table and was not handling it either. `doStoreJobsInTx` is now a single loop that handles both conditions uniformly; the sanitize retry still runs at most once. ## Bonus - `inStoreSafeCtx` no longer logs stale-ds at error level, since it's an expected retryable condition, not a failure. - When many goroutines hit a stale ds at the same time (which is exactly what happens during a dataset swap), only the first one to grab the dsList write lock actually refreshes. The rest see that the last dataset has already moved past the one they failed on and skip the refresh, going straight back to the retry. This avoids a thundering herd of redundant refreshes. ## Linear Ticket resolves PIPE-2993 ## Security - [x] The code changed/added as part of this pull request won't create any security issues with how the software is being used. <!-- GitButler Footer Boundary Top --> --- This is **part 1 of 4 in a stack** made with GitButler: - <kbd> 4 </kbd> #6979 - <kbd> 3 </kbd> #6967 - <kbd> 2 </kbd> #6962 - <kbd> 1 </kbd> #6963 👈 <!-- GitButler Footer Boundary Bottom -->
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 22, 2026 16:01
61c77d7 to
bff84b6
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 22, 2026 16:01
780eb86 to
ecb0673
Compare
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 25, 2026 06:01
bff84b6 to
6f81737
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 25, 2026 06:01
ecb0673 to
8682cd0
Compare
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 25, 2026 07:15
6f81737 to
2713e74
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 25, 2026 07:15
8682cd0 to
f798b43
Compare
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 25, 2026 12:33
2713e74 to
4a1dab5
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 25, 2026 12:33
f798b43 to
c596d8c
Compare
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 25, 2026 12:58
4a1dab5 to
418e75f
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 25, 2026 12:58
c596d8c to
753a18a
Compare
atzoum
added a commit
that referenced
this pull request
May 25, 2026
…ted datasets (#6962) 🔒 Scanned for secrets using gitleaks 8.30.1 # Description Introducing support for dropping completed dataset tables in a non-blocking fashion without a migration lock: ### 1. Versioned dataset list with reader-drain semantics A new `versionedDSList` replaces the bare `datasetList` / `datasetRangeList` fields on `Handle`. Each update bumps a version; readers acquire a snapshot tagged with that version and release it when done reading. An additional `wait(through)` API returns a channel that closes once no reader is still using a version `<= through`, so a dropper can block until in-flight queries that still see the old list have finished. ### 2. `nonBlockingCompletedDSDrop` fast path and async drop loop - A new `dropDSLoop` consumes a `dropDSList` queue. Each entry records the dslist version that must drain before the table can be dropped. - `addToDropDSList` republishes the dslist without the queued datasets, writes the persistent pre-drop marker (see §3), and pings the loop. The actual `DROP TABLE` happens off the hot path. - `doMigrateDS` gains a fast path gated by **`jobsdb.<prefix>.nonBlockingCompletedDSDrop`** (default `false`): datasets with zero pending jobs are routed through the async drop loop instead of the in-lock `postMigrateHandleDS` path. When disabled, behavior is unchanged. ### 3. Persistent `pre_drop:v1` table marker `COMMENT ON TABLE … IS 'rudder:pre_drop:v1'` is written as part of queuing a dataset for drop. This makes the queue crash-safe — the source of truth lives in Postgres, not just in-memory `dropDSList`. - `getAllTableNames` now joins `pg_class`/`pg_namespace`/`pg_description` and filters out marked tables, masking half-pairs (drops both the jobs and status table when either is marked). - New SQL migration `000019_jobsdb_functions_pre_drop_filter.up.sql` rewrites `unionjobsdb`, `unionjobsdbmetadata`, and `joborderlog` to apply the same `pg_description` filter, so server-side discovery skips marked tables too. - `cleanupPreDropTables` runs once at startup of `read` or `readwrite` jobsdbs and after journal recovery, dropping any tables left marked by a previous crashed run. If there isn't any marker present, behavior remains identical as before. ### 4. `RS001` tolerance on the `UpdateJobStatus` path The readonly-trigger error code `RS001` was previously only handled on the `Store` path. The `Update` path now wraps each `internalUpdateJobStatusInTx` call in a savepoint: on `ErrStaleDsList` it rolls back to the savepoint, refreshes the dslist under the write lock, updates the `UpdateSafeTx`'s embedded dslist/rangeList snapshot, and retries the whole command. We do **not** replay the caller-owned outer transaction. To support this, `UpdateSafeTx` now carries its own `dsList`/`dsRangeList` (with a `setDSList` setter) so retries reroute against the refreshed snapshot without re-running `inUpdateSafeCtx`. This change is harmless until a follow-up PR starts marking status tables readonly during compaction. ## Linear Ticket resolves PIPE-2996 ## Security - [x] The code changed/added as part of this pull request won't create any security issues with how the software is being used. <!-- GitButler Footer Boundary Top --> --- This is **part 2 of 4 in a stack** made with GitButler: - <kbd> 4 </kbd> #6979 - <kbd> 3 </kbd> #6967 - <kbd> 2 </kbd> #6962 👈 - <kbd> 1 </kbd> #6963 <!-- GitButler Footer Boundary Bottom -->
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 25, 2026 14:17
418e75f to
5fb92d7
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
3 times, most recently
from
May 26, 2026 07:44
f313f83 to
e410498
Compare
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 26, 2026 07:48
f9d31cb to
a538468
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 26, 2026 07:48
e410498 to
e1a8be5
Compare
atzoum
force-pushed
the
feat.nonBlockingMigration
branch
from
May 26, 2026 07:50
a538468 to
641ed1d
Compare
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 26, 2026 07:50
e1a8be5 to
d693f08
Compare
atzoum
added a commit
that referenced
this pull request
May 26, 2026
🔒 Scanned for secrets using gitleaks 8.30.1 # Description Introduces a non-blocking compaction flow for the jobsdb migration loop, gated behind the `nonBlockingCompaction` flag. When enabled it dramatically shrinks the lock window taken during dataset migration: - **`dsMigrationLock` is no longer taken.** Concurrent readers (`getJobs`, `GetPileUpCounts`, `GetDistinctParameterValues`) are not blocked by an in-flight compaction. - **Caveat:** an `UpdateJobStatus` call that targets a source dataset *while it is being compacted* will block at the PG level on the `EXCLUSIVE` lock held against that source's status table, until the compaction TX commits — bounded by the per-source `COPY` duration. After commit, late writers landing on the old status table are fenced by the readonly trigger and routed to the new destination via the existing `ErrStaleDsList` retry path. Compared to the legacy path, the writer side of `dsListLock` is reduced from "entire migration TX, including bulk `COPY` of all non-terminal jobs and `DROP TABLE` of every source" down to "`COMMIT` + a single `getDSList` + a `MIN/MAX` scan of the new destination". ## New maintenance pool Introducing a new maintenance pool which can be used by maintenance operations, such as: - adding new dataset - refreshing dataset list - compacting datasets - compacting job status tables This pool helps closing a connection-pool deadlock vector where jobsdb readers and writers could fully exhaust the pool, blocking an active maintenance goroutine while it still held a mutex (e.g. post-commit compaction which requires acquiring a new connection to refresh the dataset list). In addition, all maintenance goroutines were updated to ensure they require no more than a single connection at any time, eliminating pool-related deadlock risks caused by previously nested connection usage. If no maintenance pool is injected, the calls fall back to `dbHandle` (backwards compatibility). ## Lock stats `dsListLock` and `dsMigrationLock` now emit timing metrics, tagged by lock type (`read`/`write`) and whether the acquisition was async: | Metric | Description | |---|---| | `jobsdb_lock_wait_time` | Time spent waiting to acquire the lock | | `jobsdb_lock_time` | Time the lock was held (excluding wait) | | `jobsdb_lock_total_time` | End-to-end time (wait + hold) | All metrics carry a `name` tag so lock contention can be tracked per jobsdb instance. ## Flags - `JobsDB.<prefix>.nonBlockingCompaction` (default `false`) — gates the new flow. When off, `doMigrateDS` falls back to the legacy in-TX migrate+drop path. - `JobsDB.<prefix>.getJobsRetryOnCompaction` (default `true`) — gates the `getJobs` snapshot revalidation. When on, if a dataset that `getJobs` queried was compacted mid-read, the call returns `ErrStaleDsList` and is retried against the freshly published list. **Why:** while `getJobs` is reading the source's status table, an `UpdateJobStatus` for one of the same jobs could commit against the new dataset. Without the retry, `getJobs` would miss that status update, risking out-of-order processing downstream. This option has no effect unless `nonBlockingCompaction` is also on. ## Testing #6979 runs all tests with non-blocking partition migration enabled ## Linear Ticket resolves PIPE-2997 ## Security - [x] The code changed/added as part of this pull request won't create any security issues with how the software is being used. <!-- GitButler Footer Boundary Top --> --- This is **part 3 of 4 in a stack** made with GitButler: - <kbd> 4 </kbd> #6979 - <kbd> 3 </kbd> #6967 👈 - <kbd> 2 </kbd> #6962 - <kbd> 1 </kbd> #6963 <!-- GitButler Footer Boundary Bottom -->
🔒 Scanned for secrets using gitleaks 8.30.1 chore: timed lock
atzoum
force-pushed
the
feat.nonBlockingMigrationEnabled
branch
from
May 28, 2026 14:49
d693f08 to
781e075
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔒 Scanned for secrets using gitleaks 8.30.1
just for verifying all tests succeed with non blocking migration enabled by default
This is part 4 of 4 in a stack made with GitButler: