Skip to content

fix: reopen DuckLake catalog after restart and seed data dir ownership - #496

Merged
weicao merged 3 commits into
mainfrom
feat/task88-reattach-volume
Sep 7, 2026
Merged

fix: reopen DuckLake catalog after restart and seed data dir ownership#496
weicao merged 3 commits into
mainfrom
feat/task88-reattach-volume

Conversation

@weicao

@weicao weicao commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes two 0.3-dev image blockers from task #88 (be2cd5bd must not be promoted):

  1. Same-volume recreate failed ATTACH with stage=attach reason=driver_error. Root cause: attachCatalogs ATTACHes every *.db in the data directory as a regular DuckDB database, so on restart the DuckLake metadata file is opened first and the later ducklake: ATTACH fails. Fix: skip the configured metadata path in AttachCatalog.

    CLI ATTACH of the r5 catalog copy succeeded both with and without CREATE_IF_NOT_EXISTS when the file was not already attached as a regular database. Omitting that option on existing files is still kept (os.Stat only ErrNotExist creates; permission/IO fail closed) but is not the recreate root cause.

  2. Empty Docker named volumes mounted at /home/admin/data were root:root, so user admin (uid 1000) could not create myduck.db. The image now seeds /home/admin/data and /home/admin/log as admin.

Also documents object-table PRIMARY KEY rejection (1105, no leftover files).

Does not change v0.2.1 / latest. Does not touch PR #489.

Test

  • go test ./catalog -count=1 -run 'TestDuckLakeAttachUsesServicePaths|TestDuckLakeAttachOmitsCreateIfCatalogExists|TestAttachCatalogSkipsDuckLakeMetadataFile|TestDuckLakeAttachStatNonExistErrorDoesNotCreate|TestDuckLakeAttachRejectsRemoteCatalogURI'

Omit CREATE_IF_NOT_EXISTS when the local DuckLake catalog file already exists so same-volume recreate can ATTACH.

Create /home/admin/data and /home/admin/log as admin in the image so empty Docker named volumes inherit uid 1000.

Document object-table PRIMARY KEY rejection and named-volume ownership.

Does not change v0.2.1 / latest.

@leon-ape leon-ape left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Independent review of exact head 36ed564dcbbc6ccbfc5b40dee395acb35f0a53a9.

  • Unique parent 93d2cf9db417038f5467b5753f4e078e24e4f9ba; GitHub verification valid.
  • Existing local catalog omits CREATE_IF_NOT_EXISTS; missing catalog still sets it. Test TestDuckLakeAttachOmitsCreateIfCatalogExists locks that.
  • Dockerfile creates /home/admin/data and /home/admin/log as admin after USER admin.
  • Docs record PRIMARY KEY rejection and named-volume ownership.

APPROVE binds only this OID. Does not accept image be2cd5bd or reopen task #78. Runtime remount/first-volume still need a new immutable image after merge. No v0.2.1 / latest change.

Restart failed because attachCatalogs attached ducklake.db as a normal DuckDB database, so the later ducklake: ATTACH returned driver_error.

Skip the configured metadata file in AttachCatalog.
Stat: only ErrNotExist selects CREATE_IF_NOT_EXISTS; permission/IO errors fail closed.

CLI ATTACH of the r5 catalog succeeded both with and without CREATE_IF_NOT_EXISTS when the file was not already attached as a regular database.

@leon-ape leon-ape left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Independent review of exact head 57058f32c47684a3b5f3c26e6a95806b9bf97483.

  • Unique parent 36ed564dcbbc6ccbfc5b40dee395acb35f0a53a9; GitHub verification valid.
  • duckLakeCatalogMissing only treats os.IsNotExist as missing; other Stat errors fail closed and do not emit ATTACH.
  • AttachCatalog skips the configured DuckLake metadata file so it is not occupied as a regular database.
  • Tests: omit CREATE_IF_NOT_EXISTS when the file exists; skip metadata file; permission Stat does not create.

APPROVE binds only this OID. Does not promote be2cd5bd. Same-volume recreate and empty named-volume start still need a new immutable image after merge. No v0.2.1 / latest change.

Restart failed because attachCatalogs opened the metadata file as a regular DuckDB database. CREATE_IF_NOT_EXISTS is not the recreate cause: CLI ATTACH of the r5 catalog succeeded with and without that option when the file was not already attached.

Replace chmod 000 Stat coverage with injected os.ErrPermission so the test is stable as root.

@leon-ape leon-ape left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Independent review of exact head 4be256010814034131cf094b038047434dfc5933.

  • Unique parent 57058f32c47684a3b5f3c26e6a95806b9bf97483; GitHub verification valid.
  • Comments and PR body attribute restart failure to attaching the metadata file as a regular database; CREATE_IF_NOT_EXISTS is not the recreate cause.
  • Stat coverage injects os.ErrPermission via duckLakeStat (stable as root) and expects reason=permission_denied with no ATTACH SQL.

APPROVE binds only this OID. Does not promote be2cd5bd. Remount and empty named-volume start still need a new immutable image after merge. No v0.2.1 / latest change.

@weicao
weicao merged commit cc1d245 into main Sep 7, 2026
12 of 18 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.

2 participants