feat: add inline optimization for dir namespace#5244
feat: add inline optimization for dir namespace#5244jackye1995 merged 4 commits intolance-format:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } // Drop the guard here | ||
|
|
||
| self.manifest_dataset.reload().await?; | ||
|
|
||
| // Run inline optimization after delete | ||
| self.run_inline_optimization().await?; | ||
|
|
There was a problem hiding this comment.
Inline optimization failures break manifest operations
After inserting into the __manifest table we immediately call self.run_inline_optimization().await?; (lines 767‑772), and the same happens after deletions (lines 790‑794). The manifest row has already been merged or deleted before this call, yet any error from creating indexes, compacting files, or optimizing indices bubbles up to the caller. This makes user operations non‑atomic: a transient failure during the optional optimization causes create_table/register_table to report an error even though the entry was already added, and drop_table/deregister_table returns an error after the manifest row is gone, preventing the physical data cleanup from running and leaving orphaned files. Inline optimization should be best‑effort or rolled back; as written it can leave the namespace in an inconsistent state whenever compaction or index maintenance fails.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5244 +/- ##
==========================================
+ Coverage 82.23% 82.27% +0.03%
==========================================
Files 344 344
Lines 144765 144472 -293
Branches 144765 144472 -293
==========================================
- Hits 119050 118865 -185
+ Misses 21797 21682 -115
- Partials 3918 3925 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| log::info!( | ||
| "Created BTREE index '{}' on object_id for __manifest table", | ||
| OBJECT_ID_INDEX_NAME | ||
| ); |
There was a problem hiding this comment.
Should this be info level? Or maybe debug level. Seems like it would be noisy at info level.
There was a problem hiding this comment.
I felt info was better because this is a one time creation
Additional remaining work in lancedb/lancedb#2708
Additional remaining work in lancedb/lancedb#2708