Skip to content

SEEK-517: make catalog bootstrap concurrency-safe - #262

Merged
hnwyllmm merged 2 commits into
oceanbase:developfrom
cms-cms:fix/seek-517-catalog-bootstrap
Sep 3, 2026
Merged

SEEK-517: make catalog bootstrap concurrency-safe#262
hnwyllmm merged 2 commits into
oceanbase:developfrom
cms-cms:fix/seek-517-catalog-bootstrap

Conversation

@cms-cms

@cms-cms cms-cms commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make SDK catalog bootstrap state-driven and concurrency-safe
  • create missing tables/indexes only, then verify actual catalog state
  • retry only explicit transient DDL races with bounded backoff
  • reuse the same bootstrap logic for namespace catalogs
  • add upgrade, race, error-classification, and failure-path tests

Verification

  • original server reproducer on retained SeekDB revision: 100/100 passed
  • targeted unit tests: 162 passed
  • existing full unit suite: 437 passed, 358 skipped, 0 failed
  • Ruff and format checks passed
  • pylibseekdb package was reused, not rebuilt

Source: SEEK-517

Summary by CodeRabbit

  • Bug Fixes
    • Improved catalog initialization reliability by verifying table and index availability after setup.
    • Added bounded retries for temporary visibility issues and concurrent creation conflicts.
    • Prevented unnecessary database changes when catalog objects are already ready.
    • Catalog setup now rejects incompatible existing indexes and surfaces permission errors without retrying.
    • Namespace catalog creation completes with fewer redundant database operations while preserving required uniqueness constraints.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 9b507705-67e6-443c-acf2-1840a7075b59

📥 Commits

Reviewing files that changed from the base of the PR and between 830801f and e88e9af.

📒 Files selected for processing (2)
  • src/pyseekdb/client/client_base.py
  • tests/unit_tests/test_get_or_create_collection_concurrency.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Catalog bootstrap now uses bounded retries, error classification, rollback, and metadata re-checks. SDK and namespace setup uses shared catalog table initialization. Tests cover missing tables, legacy indexes, concurrent creation, visibility failures, permissions, and invalid indexes.

Changes

Catalog bootstrap

Layer / File(s) Summary
Bootstrap error and retry contracts
src/pyseekdb/client/client_base.py
Defines retry timing, transient database error classification, exception-chain handling, and catalog readiness signals.
Catalog table initialization
src/pyseekdb/client/client_base.py
Adds verified table and unique-index initialization for SDK and namespace catalogs. Transient failures trigger rollback and bounded backoff.
Bootstrap behavior validation
tests/unit_tests/test_get_or_create_collection_concurrency.py, tests/unit_tests/test_namespace.py
Tests ready, missing, legacy, concurrent, persistent, permission, invalid-index, wrapped-error, and catalog-order cases.

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

Merge Risk: ⚪ Minimal · up to e88e9

Catalog bootstrap now creates and verifies only missing schema objects, retries bounded transient DDL races, and shares this behavior with namespace catalogs. The supplied checks pass without an identified merge-blocking risk.

Sequence Diagram(s)

sequenceDiagram
  participant BaseClient
  participant CatalogDatabase
  participant CatalogMetadata
  BaseClient->>CatalogDatabase: Probe catalog table and index state
  BaseClient->>CatalogDatabase: Create missing table or unique index
  CatalogDatabase-->>CatalogMetadata: Expose updated catalog state
  BaseClient->>CatalogMetadata: Re-read table and index visibility
  BaseClient->>CatalogDatabase: Roll back transient failure
Loading

Suggested reviewers: hnwyllmm

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making catalog bootstrap concurrency-safe.
Docstring Coverage ✅ Passed Docstring coverage is 82.05% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 3 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@cms-cms cms-cms changed the title fix: make catalog bootstrap concurrency-safefix: make catalog bootstrap concurrency-safe SEEK-517: make catalog bootstrap concurrency-safe Sep 2, 2026
@CLAassistant

CLAassistant commented Sep 2, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cms-cms
cms-cms force-pushed the fix/seek-517-catalog-bootstrap branch from 5af977c to e88e9af Compare September 3, 2026 03:12
@hnwyllmm
hnwyllmm merged commit 33ad68a into oceanbase:develop Sep 3, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants