Fixed bug Detected loop in FindAll({0}) #2525 #2582#2603
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an infinite loop bug encountered during the rebuild process of large databases by computing the maximum items count from the source files.
- Added a new static method GetSourceMaxItemsCount in RebuildService to calculate max items based on data and log file sizes.
- Updated the Rebuild method to compute and pass the calculated maxItemsCount.
- Updated RebuildContent in the Engine to accept and use the new maxItemsCount parameter.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| LiteDB/Engine/Services/RebuildService.cs | Added GetSourceMaxItemsCount to compute max items count dynamically. |
| LiteDB/Engine/Engine/Rebuild.cs | Modified RebuildContent overload to accept and use maxItemsCount parameter. |
Comments suppressed due to low confidence (1)
LiteDB/Engine/Services/RebuildService.cs:41
- Consider adding unit tests for GetSourceMaxItemsCount to validate that the computed maximum items count correctly reflects the source file sizes. This will help ensure that the intended behavior is maintained during future changes.
private static uint GetSourceMaxItemsCount(EngineSettings settings)
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting
| foreach (var idx in reader.GetIndexes(collection)) | ||
| { | ||
| this.EnsureIndex(collection, | ||
| index.Name, | ||
| BsonExpression.Create(index.Expression), | ||
| index.Unique); | ||
| try | ||
| { | ||
| EnsureIndex(collection, | ||
| idx.Name, | ||
| BsonExpression.Create(idx.Expression), | ||
| idx.Unique); | ||
| } | ||
| catch (LiteException ex) when (ex.Message.IndexOf("Detected loop in FindAll", StringComparison.OrdinalIgnoreCase) >= 0) | ||
| { | ||
| try { DropIndex(collection, idx.Name); } catch { /* best effort */ } | ||
|
|
||
| var expr = BsonExpression.Create(idx.Expression); | ||
|
|
||
| RebuildHelpers.EnsureIndexFromDataScan( | ||
| snapshot, | ||
| idx.Name, | ||
| expr, | ||
| idx.Unique, | ||
| indexer, | ||
| data, | ||
| transaction.Safepoint | ||
| ); |
There was a problem hiding this comment.
When EnsureIndex throws with a “Detected loop in FindAll” message, it executes inside AutoTransaction, which automatically calls Rollback() and releases the current transaction because it isn’t marked explicit. The catch block then tries to recover by calling DropIndex and RebuildHelpers.EnsureIndexFromDataScan using the original transaction, snapshot, indexer and data. At this point those objects belong to a rolled back/ disposed transaction and the inserts performed above have already been undone, so the fallback either operates on invalid state or silently builds an index over an empty collection. To actually rebuild the index you need to keep the transaction alive (e.g. mark it explicit) or acquire a new transaction and snapshot after the failure before scanning the data again.
Useful? React with 👍 / 👎.
This PR fixes bug when using Rebuild() to rebuild large database with more than 2550 entries in it it fails thinking that it got an infinite loop.
Fixes:
#2525 #2582