perf: remove engine runtime reference counter

[jiangzhe]

Closes #937

Summary by CodeRabbit

• Bug Fixes
  • Improved shutdown coordination to wait for active operations, session observers, and transaction cleanup activities.
  • Sessions now remain available until all inspections and observers are released, preventing premature cleanup.
  • Enhanced shutdown diagnostics to identify blocking operations, observers, and cleanup tasks.
  • Improved handling of shutdown races, abandoned transactions, and terminal session completion.

[coderabbitai]

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

• docs/tasks/000254-remove-engine-runtime-reference-accounting.md is excluded by none and included by none

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: ace0e4c0-6852-4dd5-a4d9-4a18daa952da

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Engine shutdown now relies on packed admission state, session operation and observer blockers, and mandatory-runtime permits. EngineRef no longer contributes runtime lifetime accounting. Sessions remain registered until observers and operations release. Shutdown diagnostics report structured blocker details.

Changes

Lifecycle accounting and shutdown

Layer / File(s)	Summary
Session observer and blocker lifecycle doradb-storage/src/session.rs	Session inspection uses lifecycle admission. Observer pins increment and release per-session counts. Shutdown blockers distinguish operations from observers. Closed and abandoned sessions remain registered until ownership drains. Terminal cleanup reports lock release and registry removal separately. Tests cover observer races, retention, blocker precedence, and wakeups.
Engine shutdown and mandatory cleanup doradb-storage/src/engine.rs, doradb-storage/src/catalog/index.rs, doradb-storage/src/trx/mod.rs, doradb-storage/src/trx/sys.rs	Engine shutdown removes runtime-reference and Arc::strong_count blockers. Diagnostics report session blockers and mandatory-runtime counts. Abandoned-transaction cleanup is queued during shutdown waits. DDL and cleanup documentation describes mandatory permit accounting. Tests validate the revised shutdown behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Engine
  participant SessionRegistry
  participant SessionState
  participant MandatoryRuntime
  participant Components
  Engine->>SessionRegistry: inspect shutdown blockers
  SessionRegistry->>SessionState: classify active operation or observer
  SessionState-->>Engine: return blocker state and cleanup details
  Engine->>MandatoryRuntime: wait for accepted cleanup permits
  MandatoryRuntime-->>Engine: report drained cleanup admission
  Engine->>Components: perform reverse-order teardown

Loading

Possibly related PRs

• jiangzhe/doradb#915: Extends the session-operation coordinator used by this lifecycle change.
• jiangzhe/doradb#923: Introduces related session observer and shutdown-blocker semantics.
• jiangzhe/doradb#932: Shares mandatory-runtime lifecycle and shutdown diagnostics changes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	❓ Inconclusive	The lifecycle and observer-accounting objectives are addressed, but benchmark results required by issue #937 are not evidenced.	Provide reproducible before-and-after statement and transaction no-op measurements for both required concurrency configurations.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies removal of the engine runtime reference counter, which is the main change.
Out of Scope Changes check	✅ Passed	The code and documentation changes support engine lifecycle accounting, session observer tracking, and related shutdown behavior.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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 runtime-ref-drain

---

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.}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics -30 complexity · -14 duplication

Metric	Results
Complexity	-30
Duplication	-14

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[codecov]

Codecov Report

❌ Patch coverage is 96.96970% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.41%. Comparing base (7151941) to head (4f5f829).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
doradb-storage/src/session.rs	96.77%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #939      +/-   ##
==========================================
+ Coverage   93.37%   93.41%   +0.03%     
==========================================
  Files         149      149              
  Lines      126753   126873     +120     
==========================================
+ Hits       118358   118514     +156     
+ Misses       8395     8359      -36     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[greptile-apps]

Greptile Summary

The PR removes engine-global runtime-reference accounting and makes stable session operations, explicit observer counts, and mandatory-runtime permits authoritative for shutdown.

• Replaces EngineRef lifecycle counting with ordinary shared reachability.
• Adds per-session observer registration, release, retention, and shutdown diagnostics.
• Updates shutdown coordination, tests, and lifecycle documentation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
doradb-storage/src/engine.rs	Removes runtime-reference and strong-count shutdown checks, relying on admissions, session blockers, and mandatory permits.
doradb-storage/src/session.rs	Adds synchronized observer accounting and retains closed session state until observers drain.
doradb-storage/src/trx/mod.rs	Updates cleanup-job ownership documentation to reflect mandatory internal admission as shutdown authority.
doradb-storage/src/trx/sys.rs	Clarifies abandoned-cleanup accounting without changing cleanup submission behavior.
doradb-storage/src/catalog/index.rs	Documents that accepted index DDL is covered by its stable operation and mandatory caller permit.

_{Reviews (2): Last reviewed commit: "resolve task" | Re-trigger Greptile}