SequenceStyleGenerator generates Ids twice on SQLite

[jrauber]

The SequenceStyleGenerator produces identifiers which are not unique when the increment is happening inside a transaction that is rollbacked later on and SQLite is used.

SequenceStyleGenerator set to:

• increment_size - 2
• initial_value - 1
• optimizer - pooled

The resulting sequence is:

• Id: 1 - [A001] - Committed
• Id: 2 - [A002] - Committed
• Id: 3 - [B001] - Rollback
• Id: 4 - [C000] - Committed
• Id: 3 - [C001] - Committed
• Id: 4 - [C002] - Committed

Test-Case:

[Test]
public void SequenceStyleGeneratorGeneratesIdTwiceOnSQLiteWhenIncrementIsHappeningOnRollback()
{
	var generatedIds = new List<string>();

	using (var session = OpenSession())
	using (var transaction = session.BeginTransaction(IsolationLevel.ReadCommitted))
	{
		for (int i = 1; i <= 2; i++)
		{
			var entry = new Entity { Name = $"A" + i.ToString("000") };

			session.Save(entry);

			generatedIds.Add($"Id: {entry.Id} - [{entry.Name}] - Committed");
		}

		transaction.Commit();
	}

	using (var session = OpenSession())
	using (var transaction = session.BeginTransaction(IsolationLevel.ReadCommitted))
	{
		var entry = new Entity { Name = $"B" + 1.ToString("000") };

		session.Save(entry);

		generatedIds.Add($"Id: {entry.Id} - [{entry.Name}] - Rollback");
		
		transaction.Rollback();
	}

	Assert.Throws<GenericADOException>(() => {

		for (int i = 0; i < 5; i++)
		{
			using (var session = OpenSession())
			using (var transaction = session.BeginTransaction(IsolationLevel.ReadCommitted))
			{
				var entry = new Entity { Name = $"C" + i.ToString("000") };

				session.Save(entry);

				generatedIds.Add($"Id: {entry.Id} - [{entry.Name}] - Committed");

				transaction.Commit();
			}
		}

	});

}

[fredericDelaporte]

This is due to a SQLite limitation: it does not support concurrent writing connections. Only one connection to the database can be opened at the same time to the database in writ mode.

This causes a special case in the code here (which could be handled more gracefully by using Dialect.SupportsConcurrentWritingConnections).

So the "perform work in isolation" logic for SQLite fallback to "perform work in current transaction". If a generator increment occurs in a transaction rollbacked later, it is lost and causes the generator to be out of sync with the db.

This is what happen in your case.

It looks to me the generator cannot support your scenario with a database having such a limitation. It could check, when getting the next id, that it is coherent with its previous state and if not, increment more. But that would not fix all cases, like shutting down the application after having rollbacked a transaction having caused more than one increment, then launch a new instance and try inserting new values.

[jrauber]

Thanks for your answer. Thats what I was afraid of when i came through the code parts that you mentioned while debugging. As SQLite is used in this case as an in-memory db to do integration testing the sequence could be easily swapped with an completely memory based generator solution that only increments, never persists and resets itsself on the dispose of the fixture.
Considering the quoted documentation parts below wouldn't there be an possibility to count on the unit-of-work pattern that does not write before a flush/commit to the database? As a result the write of the current session payload and the sequence generators write should not be crashing into each other and instead be in serial order (generator first, session payload second)?

Connection-String: FullUri=file:xyz.db?mode=memory&cache=shared;Version=3;New=True;Pooling=True;

Pragma on Connection: PRAGMA journal_mode= WAL;

SQLite connections can open two kinds of transactions, read and write transactions. This is not done explicitly, a transaction is implicitly a read-transaction until it first writes to a database table, at which point it becomes a write-transaction.
At most one connection to a single shared cache may open a write transaction at any one time. This may co-exist with any number of read transactions.
https://www.sqlite.org/sharedcache.html

readers do not block writers and a writer does not block readers
http://www.sqlite.org/wal.html

Multiple processes can have the same database open at the same time. Multiple processes can be doing a SELECT at the same time. But only one process can be making changes to the database at any moment in time, however.
https://sqlite.org/faq.html#q5

[fredericDelaporte]

People can flush (explicitly or auto-flush on queries) before generating an id, in the same transaction. So with such a change those cases, currently passing, would start failing.

[fredericDelaporte]

If there is a way to detect if the connection has already a write transaction ongoing or not, the logic in the transaction factory could be improved for using another connection when the current one is not already in write mode. You may try it out by using your own transaction factory. (The default one is AdoNetWithSystemTransactionFactory, derive from it and override ExecuteWorkInIsolation with your own implementation, then specify to use your custom transaction factory in configuration.)

Note this requires the SQLite database to operate in write ahead mode, otherwise the isolated work will fail to switch to write mode because write mode needs exclusive access (no reader either) by default.

[jrauber]

Thanks again for your valuable feedback, very much appreciated. I will have a look into this solution. In addition i thought about using a different db for integration testing, firebird came into mind. But maybe thats just a new solution that will have different shortcomings. As it is already not directly sticking with the "Test as you fly, and fly as you test" principle, the work around with the custom generator might be good enough to live with as its behaviour is quite near to the real thing.

[fredericDelaporte]

Firebird has other shortcomings, sure. In NHibernate tests suit, we currently ignore 2245 tests for Firebird, versus 1445 for SQLite and 2463 for SQL Server CE.

Firebird 3 embedded mode has quite changed compared to 2.5 but finding documentation for getting it working embedded was not easy some months ago. We mainly found information about 2.5 embedded, which is not suitable for v3. You can read more about that in #639, especially in #639 (comment)
(Note that we are testing it in server mode actually, no more in embedded mode.) This PR also mention a bunch of other issues with Firebird, it could be worth reading it all.