Azure table transactional storage for distributed TM #4566
Conversation
| existing.TransactionTimestamp = s.TimeStamp; | ||
| existing.TransactionManager = s.TransactionManager; | ||
| existing.SetState(s.State, this.jsonSettings); | ||
| batchOperation.Replace(existing); |
There was a problem hiding this comment.
Under what conditions would a prepared state change?
There was a problem hiding this comment.
the prepared states are indexed by sequence id (1,2,3,4...). These sequence ids serve as the row key in the table. When a transaction is aborted, the same sequence is reused by a different transaction.
| } | ||
|
|
||
| // third, persist metadata and commit position | ||
| key.Metadata = metadata; |
There was a problem hiding this comment.
the key serves as a synchronizer, to prevent modification by multiple grains under edge conditions, like duplicate activations or deployments. Every batch write needs to include the key, even if the key values don't change.
| { | ||
| public static string MakeRowKey(long sequenceId) | ||
| { | ||
| return sequenceId.ToString("x16"); |
There was a problem hiding this comment.
Please prefix rowkey and use {RKPrefix} to KRPrefix~ range pattern for min max rowkey. This is a tested pattern we're trying to standardize on for maintainability reasons.
| this.logger = logger; | ||
| } | ||
|
|
||
| public async Task<TransactionalStorageLoadResponse<TState>> Load(string stateName) |
There was a problem hiding this comment.
statename should probably be used from the interfaces calls (load, store)
There was a problem hiding this comment.
I don't prefer including statename... cleaner to specify it in the factory, which eliminates the ugly ConcurrentDictionary inside the provider.
I actually thought you already came to the same conclusion in #4538 - you removed statename from ITransactionalStateStorage, and removed the ConcurrentDictionary.
There was a problem hiding this comment.
Looks like I did not consistently update my code to reflect the latest pattern. Will do that.
| var enumerator = batchOperation.GetEnumerator(); | ||
| while (enumerator.MoveNext()) | ||
| { | ||
| portion.Add(enumerator.Current); |
There was a problem hiding this comment.
nifty. I didn't know you could move operations from batch to batch! :)
There was a problem hiding this comment.
Yeah. But I'll have to rewrite this part to make it easier to add the key to each batch.
New implementation for Azure table transactional storage for distributed TM.
Supports arbitrarily sized batches.
Passes existing transactions tests (NOTE: fails on devstorage, must use actual Azure table).
Testing is somewhat incomplete, we still need more tests to exercise all the recovery situations.