Why I don't like FireBird: Part II
Take a look at this code. It is supposed to give me the earliest message, ensuring that I'll get each message once and only once.
It doesn't work. I am getting some messages twice. The same code (well, simplified) just works on SQLite.
public QueueMessage GetEarliestMessage() { byte[] data = null; bool done = false; while (done == false) { Transaction(cmd => { /* SELECT FIRST 1 Id, Data FROM IncomingMessages ORDER BY InsertedAt ASC */ cmd.CommandText = Queries.GetEarliestMessageFromIncomingQueue; string id; using (var reader = cmd.ExecuteReader()) { if (reader.Read() == false) { done = true; return; } id = reader.GetString(0); data = (byte[])reader[1]; } /* DELETE FROM IncomingMessages WHERE Id = @Id */ cmd.CommandText = Queries.DeleteMessageFromIncomingQueue; cmd.Parameters.Add("@Id", id); try { var rowAffected = cmd.ExecuteNonQuery(); // someone else already grabbed and deleted this row, // so we will try again with another one if (rowAffected != 1) return; // same as continue in this case} } catch (FbException e) { // yuck! it would have been better to compare the error code // but FB doesn't exposes it if (e.Message == "cannot update erased record") { return;// same as continue } } done = true;// same as break from the loop }); } if (data == null) return null; return Deserialize(data); } protected void Transaction(Action<FbCommand> action) { using (var connection = new FbConnection(connectionString)) using (var cmd = connection.CreateCommand()) { connection.Open(); using (var tx = connection.BeginTransaction(IsolationLevel.Serializable)) { cmd.Transaction = tx; action(cmd); tx.Commit(); } } }
a
Comments
Try
e.Number or e.Errors[0].Number
See:
http://www.firebirdsql.org/dotnetfirebird/documentation/api/1.7/FirebirdSql.Data.Firebird.FbException.html
That is not the issue, the issue is that there is no transaction isolation maintained
Duh, firebird doesn't have that isolation mode (only some databases do).
What I DO find disturbing in your code are:
1) a big chunk of code in a lambda. IMHO it's better to write a separate routine for that but alas
2) a fetch inside a transaction. Fetches inside transactions are a clue that something is out of order: fetch first, then start transaction.
Frans,
The code above should work with even with read committed.
Notice that I am explicitly checking that the row was removed, and only then moving on.
I still managed to get duplicate results here.
1) I often use this approach to handle additional concerns. Consider this as a poor man's AOP.
2) I disagree with that. All access to the DB should be done under a transaction.
You mean, the second select shouldn't succeed if someone else already read the row which should have placed a read lock on the row? I'm not sure if Firebird sets read-locks in such a way that multiple reads of the same row isnt possible if the row was already locked for reads.
I'm also not sure if firebird supports hints for locks. Their manual is pretty bad, to say the least.
There isn't a second select, it is a delete.
Basically, I am doing:
1) give me the first row
2) delete the previously selected row
3) if successfully deleted, return row
4) if failed to delete, go to 1)
That is safe in read committed mode, but it fails with FireBird.
Serializable and ReadCommited are both supported in FireBird. ReadCommited is default isolation.
http://www.firebirdsql.org/dotnetfirebird/blog/2005/02/transaction-isolation-levels-in.html
could you provide some really simple set of SQL/DDL commands to simulate the error behavior you describe?
You have the SQL there, and the table structure is 1:1 with that.
Make sure to run this on 3 threads, that is how I got it.
This comment:
// someone else already grabbed and deleted this row,
// so we will try again with another one
is shouting semaphore.
Even then, I'm hearing MSMQ whispering in the wind.
I'll let you guess what I am working on...
Could it be a bug in your code? If I understand correctly, in the exception catch you only test for one message, for other messages control falls through to done = true and ends the loop, returning the data that was read in the first select command.
The code isn't the best, but the error is real