Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run requests in parallel #4

Open
dcastro opened this issue Feb 15, 2022 · 4 comments · May be fixed by #129
Open

Run requests in parallel #4

dcastro opened this issue Feb 15, 2022 · 4 comments · May be fixed by #129
Assignees

Comments

@dcastro
Copy link
Member

dcastro commented Feb 15, 2022

Most commands need to retrieve all entries descending from either the root directory or some other directory.
When we have hundreds of entries stored in vault, this will mean doing hundreds of requests (1 per directory + 1 per entry).

Unfortunately, Vault's API does not support batch requests.

At the moment, these requests are done sequentially. We'd see a massive speedup if they were run concurrently.

Commands where this could be applied:

  • view / find: read directories/entries concurrently
  • rename / copy: read / write entries
  • rename / delete --recursive: delete entries
@MagicRB
Copy link
Contributor

MagicRB commented Feb 15, 2022

This should definitely be done in core, so that there are effects describing this abstract "feature", which are than implemented by each backend. We can do it in two ways imo:

  1. readSecretMany :: [Path] -> [Entry]
  2. a very complicated type thing, which will probably utilize something similar to depedent types, so that one can construct [ readSecret ..., listSecrets ...] and [writeSecret ..., deleteSecret ...] but not [ writeSecret ..., readSecret ..., ]. This would make for an API that's correct at the type level, but I'm not sure how to do it in Haskell (or in Agda for that matter, but I'm at least sure it can be done)

good idea bad idea? I'd love to implement option 2, I just have no clue how

@dcastro
Copy link
Member Author

dcastro commented Apr 29, 2022

Regarding (1):

This should definitely be done in core, so that there are effects describing this abstract "feature", which are than implemented by each backend

So, we have (will have) two backends, vault and pass.
I don't think it makes sense to generalize ReadSecret into ReadSecretMany because neither vault nor pass can retrieve more than one entry at once, right?

As far as I can tell, the same applies to WriteSecret, ListSecrets and DeleteSecret.

If that's the case, then I think it makes sense to do this at a higher-level. I.e. in the implementation of the copy command in Command.hs, call WriteSecret multiple times concurrently (I think polysemy's sequenceConcurrently would be handy here).


Regarding (2): I'm not sure I understood what you had in mind. Can you please explain it another way?

@sancho20021
Copy link
Contributor

So what is the final task? To rewrite commands in Command.hs using parallel requests? I also didn't understand what @MagicRB meant about dependent types, but if I understood correctly it is not needed, if we parallelize things in Command.hs and not on lower level of readSecrets.

@dcastro
Copy link
Member Author

dcastro commented May 5, 2022

@sancho20021 yeah let's try solving this at the Command.hs level, seems like the simplest most straightforward solution, possibly using sequenceConcurrently to parallelize where appropriate. I haven't tried it, but from a cursory glance, looks like it should work.

sancho20021 added a commit that referenced this issue May 6, 2022
Problem: some function in Commands.hs uses StateT transformer
with Sem monad.

Solution: use Polysemy State instead of StateT.
@dcastro dcastro assigned dcastro and unassigned sancho20021 May 9, 2022
@dcastro dcastro assigned Kariel-Myrr and unassigned dcastro Jun 16, 2022
Kariel-Myrr added a commit that referenced this issue Jul 19, 2022
Kariel-Myrr added a commit that referenced this issue Jul 29, 2022
Kariel-Myrr added a commit that referenced this issue Jul 30, 2022
Problem:
We want to make cmd-s concurrently

Solution:
Use `Async` effect + `sequenceConcurrent` to run read operations in concurrency
Added it for: 
- `copyCmd`
Kariel-Myrr added a commit that referenced this issue Jul 30, 2022
- renameCmd 
- deleteCmd
Kariel-Myrr added a commit that referenced this issue Jul 30, 2022
Problem:
We want to run cmd in concurrency
But StateT isn't suitable for this task

Solution:
Use MVar for tread safe operations
Rewrite existing recursive `go` function to new realities
Now we're sorting every dir in alphabetic order  to prevent non stability from concurrency
@Kariel-Myrr Kariel-Myrr linked a pull request Jul 30, 2022 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants