Blocking wallet API

[Patrik-Stas]

The wallet API is currently defined as follows:

pub trait WalletStorage {
    fn get(&self, type_: &[u8], id: &[u8], options: &str) -> Result<StorageRecord, IndyError>;
    fn add(&self, type_: &[u8], id: &[u8], value: &EncryptedValue, tags: &[Tag]) -> Result<(), IndyError>;
    fn update(&self, type_: &[u8], id: &[u8], value: &EncryptedValue) -> Result<(), IndyError>;
    fn add_tags(&self, type_: &[u8], id: &[u8], tags: &[Tag]) -> Result<(), IndyError>;
    fn update_tags(&self, type_: &[u8], id: &[u8], tags: &[Tag]) -> Result<(), IndyError>;
    fn delete_tags(&self, type_: &[u8], id: &[u8], tag_names: &[TagName]) -> Result<(), IndyError>;
    fn delete(&self, type_: &[u8], id: &[u8]) -> Result<(), IndyError>;
    fn get_storage_metadata(&self) -> Result<Vec<u8>, IndyError>;
    fn set_storage_metadata(&self, metadata: &[u8]) -> Result<(), IndyError>;
    fn get_all(&self) -> Result<Box<dyn StorageIterator>, IndyError>;
    fn search(&self, type_: &[u8], query: &language::Operator, options: Option<&str>) -> Result<Box<dyn StorageIterator>, IndyError>;
    fn close(&mut self) -> Result<(), IndyError>;
}

The API calls are all blocking - consequently many other APIs are blocking, for example create_and_store_my_did. It's quite easy to overlook when using IndySDK with the default local sqlite wallet implementation as it's quite fast. It becomes more of an issue when storage IO is slower. I am using pgsql plugin, and read operation such as WalletStorage::get takes 4ms - so for that duration, my thread is blocked. In create_and_store_my_did there's 1 read and 3 writes and so that can easily block my thread due to IO for 20ms (well, I can add few more threads to IndySDK executor, but they easily can get stuck on IO while they could had been doing some actual work instead).

I understand that the API was written quite a while ago and at the time, Rust futures were probably not yet flashed out much. I also know migrating this to async futures will be huge amount of work. But this seems as important next step to make applications built on IndySDK scalable. So I am writing this issue - gotta start somewhere right? I wonder what's other people thoughts on this.

[dhh1128]

I agree with the sentiment. Some have argued pretty emphatically that the best approach would be to add threads on top of this. I disagree, because that would disallow the use of async drivers down at the DB level, which is almost certain to yield significant performance improvements at high scale. I would do both, but make the foundation async with blocking+threads layered on top.

[Patrik-Stas]

@dhh1128 thanks for feedback, while it seems that you mostly agree, I am bit puzzled by the last sentence.

but make the foundation async with blocking+threads

Just to make sure I understand, what do you mean by async with blocking? I would be inclined to say make the foundation non-blocking async+threads layered on top.. But maybe I am misunderstanding and we mean the same thing?

Also further elaborating on my original post, some practical example.
The fact that the storage calls are blocking is producing very unexpected results for someone who doesn't dig down into Rust implementation. Because the IndySDK wrappers are generally returning Futures, and so does Javascript IndySDK, I would expect that if I do this in Javascript:

let promises = []
for (let i=0; i<1000; i++) {
   promises.push(indysdk.createAndStoreMyDid())
}
const createdDids = await Promise.all(promises)

While I would expect this to be very fast (time to compute single did+keypair itself takes 127µs on my machine., But actually because of storage blocking, the await Promise.all(promises) above results into almost serial execution, only leveraging 1 db connection the whole time.

above results into almost serial execution, only leveraging 1 db connection the whole time.

I found that only IssuerCommand::CreateAndStoreCredentialDefinition and Key Derivation are thrown onto the resizable threadpool, seems other calls are running on thread that call the library.

[dhh1128]

I believe we mean the same thing. I guess I should have added a comma to my final sentence: make the foundation async COMMA with blocking + threads layered on top.

That is, underneath it should be async, and the experiment you described should be extremely fast because of the architecture you describe. If we also want blocking behavior with threads, we should add it on top.

[Patrik-Stas]

@mikelodder7 Hi Mike, I saw you are working on proposal for KMS Storage https://github.com/hyperledger/aries-rfcs/blob/c4930d5368386637154c868e385921c4e2e5d641/concepts/0440-kms-architectures/README.md
I've also read through the LOX related content you've written. I wonder if you are aware of sort of big picture roadmap on how AriesKMS would be integrated to IndySDK (if even) and what's relationship between AriesKMS and existing IndySDK wallet implementation would be.
I see ongoing efforts to break down IndySDK to smaller chunks (IndyVDR, IndyCredX, ..) and I suppose eventually we will have many Aries KMS / LOX implementations. When that happens, I wonder if IndySDK wallet as is will become obsolete, or whether you think it can be reused.
The reason I am asking - I am not sure if it makes sense to put an effort into trying to solve issues described above. What's your take on this?

[mikelodder7]

I'm not trying to solve Indy-SDK. In my opinion, Indy-SDK wallet is one implementation of Aries Storage. There will be many different implementations of Aries Storage–Mobile, Server, Desktop, Enterprise. The point of the RFCs is to implement secure solutions so I can export my data from one solution to another.

…

On Wed, May 6, 2020 at 6:36 AM Patrik Staš ***@***.***> wrote: @mikelodder7 <https://github.com/mikelodder7> Hi Mike, I saw you are working on proposal for KMS Storage https://github.com/hyperledger/aries-rfcs/blob/c4930d5368386637154c868e385921c4e2e5d641/concepts/0440-kms-architectures/README.md I've also read through the LOX related content you've written. I wonder if you are aware of sort of big picture roadmap on how AriesKMS would be integrated to IndySDK (if even) and what's relationship between AriesKMS and existing IndySDK wallet implementation would be. I see ongoing efforts to break down IndySDK to smaller chunks (IndyVDR, IndyCredX, ..) and I suppose eventually we will have many Aries KMS / LOX implementations. When that happens, I wonder if IndySDK wallet as is will become obsolete, or whether you think it can be reused. The reason I am asking - I am not sure if it makes sense to put an effort into trying to solve issues described above. What's your take on this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2164 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFPIN6VRNPKKQLXOKOQNK3RQFKVLANCNFSM4MV67YDA> .

[tmarkovski]

This issue is slowly becoming a large problem. In an enterprise scenario, the only way to scale the wallet access is to run a Kubernetes cluster, which only works with non-sqlite wallet storage. Even then, scalability will be linear.
For some reason, I was under the impression that wallet access was a thread per wallet, but that's not the case.

@mikelodder7 Happy to discuss how that RFC can be turned into specific implementation. DIF has some specs on data contracts that can be used for standardized exchange during import/export. Perhaps we can put together a working group and start this wallet 2.0 work.

[mikelodder7]

Sure thing. Let me know when you'd like to catchup.

[Alexis-Falquier]

@tmarkovski @mikelodder7 Did anything come out of this "wallet 2.0" effort?

[jovfer]

@Alexis-Falquier #2319 will address blocking wallet API issue, please see some comments at the end of the PR