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

Batchable doesn't seem to work with anything complex #133

Closed
dispalt opened this issue Apr 14, 2018 · 5 comments
Closed

Batchable doesn't seem to work with anything complex #133

dispalt opened this issue Apr 14, 2018 · 5 comments

Comments

@dispalt
Copy link
Contributor

dispalt commented Apr 14, 2018

I've got an example that compiles but throws a head error. The only thing different from the example is the interpreter looks different (less raw) and we can't combine all the effects. In the example there are two effects getUser and getUsers, all of which can be combined. This example adds something orthogonal that cannot be be combined.

Below is an example of it throwing.

https://gist.github.com/dispalt/28ece72e3bace1dece4b1edd6e4706a8

@benhutchison
Copy link
Collaborator

Replicated issue at https://scastie.scala-lang.org/tRxoUdXNQpesymVEhkcQfw

@etorreborre
Copy link
Member

I have the full solution on my other laptop but I will try to explain and reproduce here. When you want to batch queries you have a bunch of effects returning results upon evaluation

Q[T1], Q[T2], Q[T3]

You also assume that there's a way to "batch" several of those queries into a "batched" one:

Q[Z]

Here Z is supposed to be the result of invoking the 3 previous queries. The trouble is that this value is supposed to represent the 3 results T1, T2, T3 and if we want to resume the next computations as if nothing was ever batched we need to be able to "distribute" Z back to T1, T2, T3.

Now if we come back to your example, you have queries returning (). If you aggregate all the queries into a query returning () it is impossible to "distribute" back () into the number of original () expected return values. You can solve this by adding a new case to your ADT to represent the batched calls:

case class UpdateNumFieldBatched(ts: Long, fields: Int, n: Int) extends BatchQuote[List[Unit]]

In the case class above n represent the number of batched calls.

Then:

  • in Batchable you batch the relevant calls into UpdateNumFieldBatched and update n accordingly

  • you define Z as List[Unit] and E as Unit

  • you interpret UpdateNumFieldBatched in your interpreter so that it returns List.fill(n)(())

This works but unfortunately is completely type-unsafe. Maybe we could try to define laws that a given Batchable instance + interpreter should satisfy but that doesn't seem obvious to me.

I will send you the code tonight if you cannot write it with the indications above.

@dispalt
Copy link
Contributor Author

dispalt commented Apr 16, 2018

I figured it was something like that. That makes sense, I actually ditched using it because I needed ordering I just wanted to smash a bunch of the ones next to each other, together.

Seq(
Add(1),
Add(2),
Get(2),
Add(1),
)
= Seq(Add(3), Get(2), Add(1))

So Maybe the answer should be leaving something like this comment in the source code at Batchable, it really isn't explained super well. And you're right, its not really representable in the type system, at least that I could think of.

@etorreborre
Copy link
Member

I wonder if the Batchable interface should be actually generalized to

trait Batchable[T[_]] {
  def batch(ts: List[T[Any]]): List[(List[T[Any]], T[Any])]
  def distribute(results: List[Any]): List[List[Any]]
}

Super untyped but that leaves you the opportunity to batch exactly what you want:

  • batch groups a list of commands into one
  • distribute distributes each results to a list of results for each original command

But the opportunity for making mistakes is so big! And also very coupled to how commands are being interpreted. Which makes me this that maybe this is better left to each interpreter to do this dispatch/distribution internally by privately creating batched commands and redistributing results. So my last thought is: maybe I should just drop the functionality and give an example of a batching interpreter?

@etorreborre
Copy link
Member

I am closing to close this for now until we can come up with something more sensible.

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

No branches or pull requests

3 participants