mbdavid / LiteDB

LiteDB - A .NET NoSQL Document Store in a single data file
http://www.litedb.org
MIT License
8.52k stars 1.24k forks source link

[BUG] Database log file not being cleaned up after using FindOne() #2440

Closed teknynja closed 3 months ago

teknynja commented 7 months ago

Version v5.0.19

Describe the bug Using FindOne() is holding a read lock and open cursor because it is not completely iterating the return result from the delegated Find() call. This results in the the Close()/Dispose() methods skipping the cleanup (and failing to merge the pending transactions!)

I can work around this issue by using Find(...).ToArray().First() instead of FindOne(...) but that is an ugly hack and could be very inefficient if the predicate returns a large number of results.

This was not an issue in v5.0.17, and I first noticed it on 5.0.18. I thought the fix for #2435 in v5.0.19 might address this problem but it still persists. I'm surprised that there aren't other reports of lingering log files since the release of v5.0.18?

Code to Reproduce

using LiteDB;

namespace litedb_bug {
    internal class Program {
        const string DATABASE_FILENAME = "test.db";

        static void Main(string[] args) {

            CreateDatabase();

            using (LiteDatabase database = new LiteDatabase(DATABASE_FILENAME)) {

                ILiteCollection<Record> collection = database.GetCollection<Record>("records");

                /// Using `FindOne()` holds read lock because it doesn't finish iterating the
                ///  delegated `Find()` call:
                Record bob = collection.FindOne(x => x.Name == "Bob");

                /// Using `Find().ToArray().First()` is a horrible workaround but **does**
                ///  release the read lock by completing the iteration:
                // Record bob = collection.Find(x => x.Name == "Bob").ToArray().First();

                bob.Timestamp = DateTimeOffset.UtcNow.ToUnixTimeSeconds();
                collection.Update(bob);

                /// Normally you wouldn't need to do this, but with the held read lock
                ///  `Dispose()` doesn't clean up the database log file. Using
                ///  `Checkpoint()` to try and force the clean up throws a
                ///  LockRecursionException because of the open read lock from above:
                database.Checkpoint();

            }
        }

        private static void CreateDatabase() {
            using (LiteDatabase database = new LiteDatabase(DATABASE_FILENAME)) {
                ILiteCollection<Record> collection = database.GetCollection<Record>("records");
                collection.DeleteAll();
                collection.Insert(new Record("Bob"));
            }
        }
    }

    public class Record {
        [BsonId] public ObjectId _id { get; set; } = ObjectId.Empty;

        public string Name { get; set; } = "";
        public long Timestamp { get; set; }

        public Record(string name) {
            Name = name;
        }
    }
}

Expected behavior FindOne() should release locks and cursors when it completes, and the log file should be merged and deleted on disposing the database.

teknynja commented 7 months ago

Note that this will probably also be an issue if an end-user abandons iterating a Find(...) result as well...

SpaceCheetah commented 7 months ago

I ran into this issue too as an end user (with FindById, it also left an open transaction and an open cursor). Using Find(f => f.Id == ...).FirstOrDefault() had the same issue. .LastOrDefault instead worked in my case. The transactions not being cleaned up were the primary issue in my case, but I did notice it leaving open cursors when I was testing workarounds.

jdtkw commented 6 months ago

It would be interesting @teknynja if you can roll in #2436 on top of the v5.0.19 codebase to see if it resolves your problem. Note that I've done the same and it looks like it resolves #2435 for us, but it is definitely not present in v5.0.19.

@mbdavid would appreciate a new build that includes #2436, as we are looking for an official build that includes this fix, and our application's release date is coming soon.

teknynja commented 6 months ago

Unfortunately, I've switched to a different DB solution and probably won't be able to pursue this issue further.

alexbereznikov commented 6 months ago

I reproduced this issue using this simple test

[Fact]
public void TestTest()
{
    var data = DataGen.Person(1, 100).ToArray();

    using (var db = new LiteDatabase("filename=:memory:"))
    {
        var person = db.GetCollection<Person>();

        person.Insert(data);

        db.BeginTrans();

        var results = person.FindAll();
        var result = results.First();

        db.Commit();
    }
}

And I can confirm that #2436 fixes it.

JKamsker commented 3 months ago

Thank you for reporting this issue.

This issue has been resolved in the latest version of LiteDB. Please update to the latest version of LiteDB to resolve this issue. If the issue persists, please let us know.